From b475068f894b95674454cc08dc327a45d218a72f Mon Sep 17 00:00:00 2001 From: Oleksandr Gorkun Date: Thu, 19 Jul 2018 11:19:10 +0300 Subject: [PATCH] MAGETWO-88431: Add mandatory form key validation to front controller --- .../Magento/Backend/App/AbstractAction.php | 7 +- .../Backend/App/Request/BackendValidator.php | 180 ++++ app/code/Magento/Backend/etc/adminhtml/di.xml | 2 + .../Controller/Account/CreatePost.php | 46 +- .../Customer/Controller/Account/EditPost.php | 38 +- .../Customer/Controller/Account/Login.php | 3 +- .../Customer/Controller/Account/LoginPost.php | 32 +- .../Customer/Controller/Account/Logout.php | 3 +- .../Unit/Controller/Account/EditPostTest.php | 795 ------------------ .../Integration/Controller/Token/Access.php | 23 +- .../Integration/Controller/Token/Request.php | 24 +- .../Observer/RegisterFormKeyFromCookie.php | 97 --- .../Plugin/RegisterFormKeyFromCookie.php | 107 +++ .../RegisterFormKeyFromCookieTest.php | 169 ---- app/code/Magento/PageCache/etc/di.xml | 3 + .../Magento/PageCache/etc/frontend/events.xml | 12 - .../StorefrontDeletePersistedWishlistTest.xml | 2 + app/etc/di.xml | 2 + .../CurlTransport/FrontendDecorator.php | 2 +- .../TestCase/AbstractController.php | 13 +- .../App/Request/BackendValidatorTest.php | 433 ++++++++++ .../Customer/Controller/AccountTest.php | 1 + .../App/Request/CsrfValidatorTest.php | 269 ++++++ .../Plugin/RegisterFormKeyFromCookieTest.php | 68 ++ .../App/CsrfAwareActionInterface.php | 39 + .../Magento/Framework/App/FrontController.php | 85 +- .../Framework/App/Request/CsrfValidator.php | 132 +++ .../App/Request/InvalidRequestException.php | 60 ++ .../App/Request/ValidatorInterface.php | 32 + lib/web/mage/common.js | 20 + 30 files changed, 1594 insertions(+), 1105 deletions(-) create mode 100644 app/code/Magento/Backend/App/Request/BackendValidator.php delete mode 100644 app/code/Magento/Customer/Test/Unit/Controller/Account/EditPostTest.php delete mode 100644 app/code/Magento/PageCache/Observer/RegisterFormKeyFromCookie.php create mode 100644 app/code/Magento/PageCache/Plugin/RegisterFormKeyFromCookie.php delete mode 100644 app/code/Magento/PageCache/Test/Unit/Observer/RegisterFormKeyFromCookieTest.php delete mode 100644 app/code/Magento/PageCache/etc/frontend/events.xml create mode 100644 dev/tests/integration/testsuite/Magento/Backend/App/Request/BackendValidatorTest.php create mode 100644 dev/tests/integration/testsuite/Magento/Framework/App/Request/CsrfValidatorTest.php create mode 100644 dev/tests/integration/testsuite/Magento/PageCache/Plugin/RegisterFormKeyFromCookieTest.php create mode 100644 lib/internal/Magento/Framework/App/CsrfAwareActionInterface.php create mode 100644 lib/internal/Magento/Framework/App/Request/CsrfValidator.php create mode 100644 lib/internal/Magento/Framework/App/Request/InvalidRequestException.php create mode 100644 lib/internal/Magento/Framework/App/Request/ValidatorInterface.php diff --git a/app/code/Magento/Backend/App/AbstractAction.php b/app/code/Magento/Backend/App/AbstractAction.php index 3f658ee90bf4e..fb2daa283f111 100644 --- a/app/code/Magento/Backend/App/AbstractAction.php +++ b/app/code/Magento/Backend/App/AbstractAction.php @@ -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()) { @@ -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() { diff --git a/app/code/Magento/Backend/App/Request/BackendValidator.php b/app/code/Magento/Backend/App/Request/BackendValidator.php new file mode 100644 index 0000000000000..878f9cb4dc4c1 --- /dev/null +++ b/app/code/Magento/Backend/App/Request/BackendValidator.php @@ -0,0 +1,180 @@ +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); + } + } + } +} diff --git a/app/code/Magento/Backend/etc/adminhtml/di.xml b/app/code/Magento/Backend/etc/adminhtml/di.xml index e08d4ac202756..d8e9674d2b4cb 100644 --- a/app/code/Magento/Backend/etc/adminhtml/di.xml +++ b/app/code/Magento/Backend/etc/adminhtml/di.xml @@ -14,6 +14,8 @@ + Magento\Framework\App\Config\ScopeConfigInterface::SCOPE_TYPE_DEFAULT diff --git a/app/code/Magento/Customer/Controller/Account/CreatePost.php b/app/code/Magento/Customer/Controller/Account/CreatePost.php index 27d8ddd99344c..bb94063226f41 100644 --- a/app/code/Magento/Customer/Controller/Account/CreatePost.php +++ b/app/code/Magento/Customer/Controller/Account/CreatePost.php @@ -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; @@ -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 @@ -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 * @@ -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(); @@ -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)); } /** diff --git a/app/code/Magento/Customer/Controller/Account/EditPost.php b/app/code/Magento/Customer/Controller/Account/EditPost.php index a10795533a2a5..aa5e088f9c892 100644 --- a/app/code/Magento/Customer/Controller/Account/EditPost.php +++ b/app/code/Magento/Customer/Controller/Account/EditPost.php @@ -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; @@ -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 @@ -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 * @@ -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; } /** diff --git a/app/code/Magento/Customer/Controller/Account/Login.php b/app/code/Magento/Customer/Controller/Account/Login.php index 51c244ec0cfe9..d685191bf43b5 100644 --- a/app/code/Magento/Customer/Controller/Account/Login.php +++ b/app/code/Magento/Customer/Controller/Account/Login.php @@ -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 diff --git a/app/code/Magento/Customer/Controller/Account/LoginPost.php b/app/code/Magento/Customer/Controller/Account/LoginPost.php index 31e2a3aeca9e3..49a3f95379d4b 100644 --- a/app/code/Magento/Customer/Controller/Account/LoginPost.php +++ b/app/code/Magento/Customer/Controller/Account/LoginPost.php @@ -11,17 +11,23 @@ use Magento\Customer\Model\Session; use Magento\Customer\Api\AccountManagementInterface; use Magento\Customer\Model\Url as CustomerUrl; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\Request\InvalidRequestException; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\EmailNotConfirmedException; use Magento\Framework\Exception\AuthenticationException; use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Exception\State\UserLockedException; use Magento\Framework\App\Config\ScopeConfigInterface; +use Magento\Customer\Controller\AbstractAccount; +use Magento\Framework\Phrase; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ -class LoginPost extends \Magento\Customer\Controller\AbstractAccount +class LoginPost extends AbstractAccount implements CsrfAwareActionInterface { /** * @var \Magento\Customer\Api\AccountManagementInterface @@ -131,6 +137,30 @@ private function getCookieMetadataFactory() return $this->cookieMetadataFactory; } + /** + * @inheritDoc + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException { + /** @var Redirect $resultRedirect */ + $resultRedirect = $this->resultRedirectFactory->create(); + $resultRedirect->setPath('*/*/'); + + return new InvalidRequestException( + $resultRedirect, + [new Phrase('Invalid Form Key. Please refresh the page.')] + ); + } + + /** + * @inheritDoc + */ + public function validateForCsrf(RequestInterface $request): ?bool + { + return null; + } + /** * Login post action * diff --git a/app/code/Magento/Customer/Controller/Account/Logout.php b/app/code/Magento/Customer/Controller/Account/Logout.php index 3d5d5480c502b..19dabf9effa56 100644 --- a/app/code/Magento/Customer/Controller/Account/Logout.php +++ b/app/code/Magento/Customer/Controller/Account/Logout.php @@ -11,8 +11,9 @@ use Magento\Framework\App\ObjectManager; use Magento\Framework\Stdlib\Cookie\CookieMetadataFactory; use Magento\Framework\Stdlib\Cookie\PhpCookieManager; +use Magento\Customer\Controller\AbstractAccount; -class Logout extends \Magento\Customer\Controller\AbstractAccount +class Logout extends AbstractAccount { /** * @var Session diff --git a/app/code/Magento/Customer/Test/Unit/Controller/Account/EditPostTest.php b/app/code/Magento/Customer/Test/Unit/Controller/Account/EditPostTest.php deleted file mode 100644 index aaa1c17027928..0000000000000 --- a/app/code/Magento/Customer/Test/Unit/Controller/Account/EditPostTest.php +++ /dev/null @@ -1,795 +0,0 @@ -prepareContext(); - - $this->customerSession = $this->getMockBuilder(\Magento\Customer\Model\Session::class) - ->disableOriginalConstructor() - ->setMethods(['getCustomerId', 'setCustomerFormData', 'logout', 'start']) - ->getMock(); - - $this->customerAccountManagement = $this->getMockBuilder(\Magento\Customer\Model\AccountManagement::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->customerRepository = $this->getMockBuilder(\Magento\Customer\Api\CustomerRepositoryInterface::class) - ->getMockForAbstractClass(); - - $this->validator = $this->getMockBuilder(\Magento\Framework\Data\Form\FormKey\Validator::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->customerExtractor = $this->getMockBuilder(\Magento\Customer\Model\CustomerExtractor::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->emailNotification = $this->getMockBuilder(EmailNotificationInterface::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->authenticationMock = $this->getMockBuilder(AuthenticationInterface::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->customerMapperMock = $this->getMockBuilder(\Magento\Customer\Model\Customer\Mapper::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->model = new EditPost( - $this->context, - $this->customerSession, - $this->customerAccountManagement, - $this->customerRepository, - $this->validator, - $this->customerExtractor - ); - - $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); - $objectManager->setBackwardCompatibleProperty( - $this->model, - 'emailNotification', - $this->emailNotification - ); - - $objectManager->setBackwardCompatibleProperty( - $this->model, - 'authentication', - $this->authenticationMock - ); - $objectManager->setBackwardCompatibleProperty( - $this->model, - 'customerMapper', - $this->customerMapperMock - ); - } - - public function testInvalidFormKey() - { - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(false); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('*/*/edit') - ->willReturnSelf(); - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - public function testNoPostValues() - { - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('isPost') - ->willReturn(false); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('*/*/edit') - ->willReturnSelf(); - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - public function testGeneralSave() - { - $customerId = 1; - $currentPassword = '1234567'; - $customerEmail = 'customer@example.com'; - - $address = $this->getMockBuilder(\Magento\Customer\Api\Data\AddressInterface::class) - ->getMockForAbstractClass(); - $currentCustomerMock = $this->getCurrentCustomerMock($customerId, $address); - $newCustomerMock = $this->getNewCustomerMock($customerId, $address); - - $currentCustomerMock->expects($this->any()) - ->method('getEmail') - ->willReturn($customerEmail); - - $this->customerMapperMock->expects($this->once()) - ->method('toFlatArray') - ->with($currentCustomerMock) - ->willReturn([]); - - $this->customerSession->expects($this->once()) - ->method('getCustomerId') - ->willReturn($customerId); - - $this->customerRepository->expects($this->once()) - ->method('getById') - ->with($customerId) - ->willReturn($currentCustomerMock); - - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('isPost') - ->willReturn(true); - - $this->request->expects($this->exactly(3)) - ->method('getParam') - ->withConsecutive( - ['change_email'], - ['change_email'], - ['change_password'] - ) - ->willReturnOnConsecutiveCalls(true, true, false); - - $this->request->expects($this->once()) - ->method('getPost') - ->with('current_password') - ->willReturn($currentPassword); - - $this->customerRepository->expects($this->once()) - ->method('getById') - ->with($customerId) - ->willReturn($currentCustomerMock); - - $this->customerRepository->expects($this->once()) - ->method('save') - ->with($newCustomerMock) - ->willReturnSelf(); - - $this->customerExtractor->expects($this->once()) - ->method('extract') - ->with('customer_account_edit', $this->request) - ->willReturn($newCustomerMock); - - $this->emailNotification->expects($this->once()) - ->method('credentialsChanged') - ->with($currentCustomerMock, $customerEmail, false) - ->willReturnSelf(); - - $newCustomerMock->expects($this->once()) - ->method('getEmail') - ->willReturn($customerEmail); - - $this->eventManager->expects($this->once()) - ->method('dispatch') - ->with( - 'customer_account_edited', - ['email' => $customerEmail] - ); - - $this->messageManager->expects($this->once()) - ->method('addSuccess') - ->with(__('You saved the account information.')) - ->willReturnSelf(); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('customer/account') - ->willReturnSelf(); - - $this->authenticationMock->expects($this->once()) - ->method('authenticate') - ->willReturn(true); - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - /** - * @param int $testNumber - * @param string $exceptionClass - * @param string $errorMessage - * - * @dataProvider changeEmailExceptionDataProvider - */ - public function testChangeEmailException($testNumber, $exceptionClass, $errorMessage) - { - $customerId = 1; - $password = '1234567'; - - $address = $this->getMockBuilder(\Magento\Customer\Api\Data\AddressInterface::class) - ->getMockForAbstractClass(); - - $currentCustomerMock = $this->getCurrentCustomerMock($customerId, $address); - $newCustomerMock = $this->getNewCustomerMock($customerId, $address); - - $this->customerMapperMock->expects($this->once()) - ->method('toFlatArray') - ->with($currentCustomerMock) - ->willReturn([]); - - $this->customerExtractor->expects($this->once()) - ->method('extract') - ->with('customer_account_edit', $this->request) - ->willReturn($newCustomerMock); - - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('isPost') - ->willReturn(true); - - $this->customerSession->expects($this->once()) - ->method('getCustomerId') - ->willReturn($customerId); - - $this->customerRepository->expects($this->once()) - ->method('getById') - ->with($customerId) - ->willReturn($currentCustomerMock); - - $this->request->expects($this->any()) - ->method('getParam') - ->with('change_email') - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('getPost') - ->with('current_password') - ->willReturn($password); - - $exception = new $exceptionClass($errorMessage); - $this->authenticationMock->expects($this->once()) - ->method('authenticate') - ->willThrowException($exception); - - $this->messageManager->expects($this->once()) - ->method('addError') - ->with($errorMessage) - ->willReturnSelf(); - - if ($testNumber==1) { - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('*/*/edit') - ->willReturnSelf(); - } - - if ($testNumber==2) { - $this->customerSession->expects($this->once()) - ->method('logout'); - - $this->customerSession->expects($this->once()) - ->method('start'); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('customer/account/login') - ->willReturnSelf(); - } - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - /** - * @return array - */ - public function changeEmailExceptionDataProvider() - { - return [ - [ - 'testNumber' => 1, - 'exceptionClass' => \Magento\Framework\Exception\InvalidEmailOrPasswordException::class, - 'errorMessage' => __("The password doesn't match this account. Verify the password and try again.") - ], - [ - 'testNumber' => 2, - 'exceptionClass' => \Magento\Framework\Exception\State\UserLockedException::class, - 'errorMessage' => __('The account sign-in was incorrect or your account is disabled temporarily. ' - . 'Please wait and try again later.') - ] - ]; - } - - /** - * @param string $currentPassword - * @param string $newPassword - * @param string $confirmationPassword - * @param [] $errors - * - * @dataProvider changePasswordDataProvider - */ - public function testChangePassword( - $currentPassword, - $newPassword, - $confirmationPassword, - $errors - ) { - $customerId = 1; - $customerEmail = 'user1@example.com'; - - $address = $this->getMockBuilder(\Magento\Customer\Api\Data\AddressInterface::class) - ->getMockForAbstractClass(); - - $currentCustomerMock = $this->getCurrentCustomerMock($customerId, $address); - $newCustomerMock = $this->getNewCustomerMock($customerId, $address); - - $this->customerMapperMock->expects($this->once()) - ->method('toFlatArray') - ->with($currentCustomerMock) - ->willReturn([]); - - $this->customerSession->expects($this->once()) - ->method('getCustomerId') - ->willReturn($customerId); - - $this->customerRepository->expects($this->once()) - ->method('getById') - ->with($customerId) - ->willReturn($currentCustomerMock); - - $this->customerExtractor->expects($this->once()) - ->method('extract') - ->with('customer_account_edit', $this->request) - ->willReturn($newCustomerMock); - - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('isPost') - ->willReturn(true); - - $this->request->expects($this->exactly(3)) - ->method('getParam') - ->withConsecutive( - ['change_email'], - ['change_email'], - ['change_password'] - ) - ->willReturnOnConsecutiveCalls(false, false, true); - - $this->request->expects($this->any()) - ->method('getPostValue') - ->willReturn(true); - - $this->request->expects($this->exactly(3)) - ->method('getPost') - ->willReturnMap([ - ['current_password', null, $currentPassword], - ['password', null, $newPassword], - ['password_confirmation', null, $confirmationPassword], - ]); - - $currentCustomerMock->expects($this->any()) - ->method('getEmail') - ->willReturn($customerEmail); - - // Prepare errors processing - if ($errors['counter'] > 0) { - $this->mockChangePasswordErrors($currentPassword, $newPassword, $errors, $customerEmail); - } else { - $this->customerAccountManagement->expects($this->once()) - ->method('changePassword') - ->with($customerEmail, $currentPassword, $newPassword) - ->willReturnSelf(); - - $this->customerRepository->expects($this->once()) - ->method('save') - ->with($newCustomerMock) - ->willReturnSelf(); - - $this->messageManager->expects($this->once()) - ->method('addSuccess') - ->with(__('You saved the account information.')) - ->willReturnSelf(); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('customer/account') - ->willReturnSelf(); - } - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - /** - * @return array - */ - public function changePasswordDataProvider() - { - return [ - [ - 'current_password' => '', - 'new_password' => '', - 'confirmation_password' => '', - 'errors' => [ - 'counter' => 1, - 'message' => __('Please enter new password.'), - ] - ], - [ - 'current_password' => '', - 'new_password' => 'user2@example.com', - 'confirmation_password' => 'user3@example.com', - 'errors' => [ - 'counter' => 1, - 'message' => __('Password confirmation doesn\'t match entered password.'), - ] - ], - [ - 'current_password' => 'user1@example.com', - 'new_password' => 'user2@example.com', - 'confirmation_password' => 'user2@example.com', - 'errors' => [ - 'counter' => 0, - 'message' => '', - ] - ], - [ - 'current_password' => 'user1@example.com', - 'new_password' => 'user2@example.com', - 'confirmation_password' => 'user2@example.com', - 'errors' => [ - 'counter' => 1, - 'message' => 'AuthenticationException', - 'exception' => \Magento\Framework\Exception\AuthenticationException::class, - ] - ], - [ - 'current_password' => 'user1@example.com', - 'new_password' => 'user2@example.com', - 'confirmation_password' => 'user2@example.com', - 'errors' => [ - 'counter' => 1, - 'message' => 'Exception', - 'exception' => '\Exception', - ] - ] - ]; - } - - /** - * @param string $message - * @param string $exception - * - * @dataProvider exceptionDataProvider - */ - public function testGeneralException( - $message, - $exception - ) { - $customerId = 1; - - $address = $this->getMockBuilder(\Magento\Customer\Api\Data\AddressInterface::class) - ->getMockForAbstractClass(); - - $currentCustomerMock = $this->getCurrentCustomerMock($customerId, $address); - $newCustomerMock = $this->getNewCustomerMock($customerId, $address); - - $this->customerMapperMock->expects($this->once()) - ->method('toFlatArray') - ->with($currentCustomerMock) - ->willReturn([]); - - $exception = new $exception(__($message)); - - $this->validator->expects($this->once()) - ->method('validate') - ->with($this->request) - ->willReturn(true); - - $this->request->expects($this->once()) - ->method('isPost') - ->willReturn(true); - - $this->request->expects($this->exactly(3)) - ->method('getParam') - ->withConsecutive( - ['change_email'], - ['change_email'], - ['change_password'] - ) - ->willReturn(false); - - $this->request->expects($this->any()) - ->method('getPostValue') - ->willReturn(true); - - $this->customerSession->expects($this->once()) - ->method('getCustomerId') - ->willReturn($customerId); - $this->customerSession->expects($this->once()) - ->method('setCustomerFormData') - ->with(true) - ->willReturnSelf(); - - $this->customerRepository->expects($this->once()) - ->method('getById') - ->with($customerId) - ->willReturn($currentCustomerMock); - $this->customerRepository->expects($this->once()) - ->method('save') - ->with($newCustomerMock) - ->willThrowException($exception); - - $this->customerExtractor->expects($this->once()) - ->method('extract') - ->with('customer_account_edit', $this->request) - ->willReturn($newCustomerMock); - - $this->resultRedirect->expects($this->once()) - ->method('setPath') - ->with('*/*/edit') - ->willReturnSelf(); - - $this->assertSame($this->resultRedirect, $this->model->execute()); - } - - /** - * @return array - */ - public function exceptionDataProvider() - { - return [ - [ - 'message' => 'LocalizedException', - 'exception' => \Magento\Framework\Exception\LocalizedException::class, - ], - [ - 'message' => 'Exception', - 'exception' => '\Exception', - ], - ]; - } - - protected function prepareContext() - { - $this->context = $this->getMockBuilder(\Magento\Framework\App\Action\Context::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->resultRedirectFactory = $this->getMockBuilder( - \Magento\Framework\Controller\Result\RedirectFactory::class - ) - ->disableOriginalConstructor() - ->setMethods(['create']) - ->getMock(); - - $this->resultRedirect = $this->getMockBuilder(\Magento\Framework\Controller\Result\Redirect::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->request = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->messageManager = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class) - ->getMockForAbstractClass(); - - $this->context->expects($this->any()) - ->method('getResultRedirectFactory') - ->willReturn($this->resultRedirectFactory); - - $this->context->expects($this->any()) - ->method('getRequest') - ->willReturn($this->request); - - $this->context->expects($this->any()) - ->method('getMessageManager') - ->willReturn($this->messageManager); - - $this->eventManager = $this->getMockBuilder(\Magento\Framework\Event\ManagerInterface::class) - ->getMockForAbstractClass(); - - $this->context->expects($this->any()) - ->method('getEventManager') - ->willReturn($this->eventManager); - - $this->resultRedirectFactory->expects($this->any()) - ->method('create') - ->willReturn($this->resultRedirect); - } - - /** - * @param int $customerId - * @param \PHPUnit_Framework_MockObject_MockObject $address - * @return \PHPUnit_Framework_MockObject_MockObject - */ - protected function getNewCustomerMock($customerId, $address) - { - $newCustomerMock = $this->getMockBuilder(\Magento\Customer\Api\Data\CustomerInterface::class) - ->getMockForAbstractClass(); - - $newCustomerMock->expects($this->once()) - ->method('setId') - ->with($customerId) - ->willReturnSelf(); - $newCustomerMock->expects($this->once()) - ->method('getAddresses') - ->willReturn(null); - $newCustomerMock->expects($this->once()) - ->method('setAddresses') - ->with([$address]) - ->willReturn(null); - - return $newCustomerMock; - } - - /** - * @param int $customerId - * @param \PHPUnit_Framework_MockObject_MockObject $address - * @return \PHPUnit_Framework_MockObject_MockObject - */ - protected function getCurrentCustomerMock($customerId, $address) - { - $currentCustomerMock = $this->getMockBuilder(\Magento\Customer\Api\Data\CustomerInterface::class) - ->getMockForAbstractClass(); - - $currentCustomerMock->expects($this->once()) - ->method('getAddresses') - ->willReturn([$address]); - - $currentCustomerMock->expects($this->any()) - ->method('getId') - ->willReturn($customerId); - - return $currentCustomerMock; - } - - /** - * @param string $currentPassword - * @param string $newPassword - * @param [] $errors - * @param string $customerEmail - * @return void - */ - protected function mockChangePasswordErrors($currentPassword, $newPassword, $errors, $customerEmail) - { - if (!empty($errors['exception'])) { - $exception = new $errors['exception'](__($errors['message'])); - - $this->customerAccountManagement->expects($this->once()) - ->method('changePassword') - ->with($customerEmail, $currentPassword, $newPassword) - ->willThrowException($exception); - - $this->messageManager->expects($this->any()) - ->method('addException') - ->with($exception, __('We can\'t save the customer.')) - ->willReturnSelf(); - } - - $this->customerSession->expects($this->once()) - ->method('setCustomerFormData') - ->with(true) - ->willReturnSelf(); - - $this->messageManager->expects($this->any()) - ->method('addError') - ->with($errors['message']) - ->willReturnSelf(); - - $this->resultRedirect->expects($this->any()) - ->method('setPath') - ->with('*/*/edit') - ->willReturnSelf(); - } -} diff --git a/app/code/Magento/Integration/Controller/Token/Access.php b/app/code/Magento/Integration/Controller/Token/Access.php index 88d5029a724c7..5b75643f51b50 100644 --- a/app/code/Magento/Integration/Controller/Token/Access.php +++ b/app/code/Magento/Integration/Controller/Token/Access.php @@ -6,11 +6,15 @@ */ namespace Magento\Integration\Controller\Token; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\Request\InvalidRequestException; +use Magento\Framework\App\RequestInterface; use Magento\Integration\Model\Integration as IntegrationModel; use Magento\Integration\Api\IntegrationServiceInterface as IntegrationService; use Magento\Integration\Api\OauthServiceInterface as IntegrationOauthService; +use Magento\Framework\App\Action\Action; -class Access extends \Magento\Framework\App\Action\Action +class Access extends Action implements CsrfAwareActionInterface { /** * @var \Magento\Framework\Oauth\OauthInterface @@ -53,6 +57,23 @@ public function __construct( $this->helper = $helper; } + /** + * @inheritDoc + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException { + return null; + } + + /** + * @inheritDoc + */ + public function validateForCsrf(RequestInterface $request): ?bool + { + return true; + } + /** * Initiate AccessToken request operation * diff --git a/app/code/Magento/Integration/Controller/Token/Request.php b/app/code/Magento/Integration/Controller/Token/Request.php index 104a36e8ae8fe..04f368fc9f9fd 100644 --- a/app/code/Magento/Integration/Controller/Token/Request.php +++ b/app/code/Magento/Integration/Controller/Token/Request.php @@ -6,7 +6,12 @@ */ namespace Magento\Integration\Controller\Token; -class Request extends \Magento\Framework\App\Action\Action +use Magento\Framework\App\Action\Action; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\Request\InvalidRequestException; +use Magento\Framework\App\RequestInterface; + +class Request extends Action implements CsrfAwareActionInterface { /** * @var \Magento\Framework\Oauth\OauthInterface @@ -33,6 +38,23 @@ public function __construct( $this->helper = $helper; } + /** + * @inheritDoc + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException { + return null; + } + + /** + * @inheritDoc + */ + public function validateForCsrf(RequestInterface $request): ?bool + { + return true; + } + /** * Initiate RequestToken request operation * diff --git a/app/code/Magento/PageCache/Observer/RegisterFormKeyFromCookie.php b/app/code/Magento/PageCache/Observer/RegisterFormKeyFromCookie.php deleted file mode 100644 index 4b51a9cf233a1..0000000000000 --- a/app/code/Magento/PageCache/Observer/RegisterFormKeyFromCookie.php +++ /dev/null @@ -1,97 +0,0 @@ -cookieFormKey = $formKey; - $this->escaper = $escaper; - $this->sessionFormKey = $sessionFormKey; - $this->cookieMetadataFactory = $cookieMetadataFactory; - $this->sessionConfig = $sessionConfig; - } - - /** - * Register form key in session from cookie value - * - * @param \Magento\Framework\Event\Observer $observer - * @return void - * @SuppressWarnings(PHPMD.UnusedFormalParameter) - */ - public function execute(\Magento\Framework\Event\Observer $observer) - { - if ($this->cookieFormKey->get()) { - $this->updateCookieFormKey($this->cookieFormKey->get()); - - $this->sessionFormKey->set( - $this->escaper->escapeHtml($this->cookieFormKey->get()) - ); - } - } - - /** - * @param string $formKey - * @return void - */ - private function updateCookieFormKey($formKey) - { - $cookieMetadata = $this->cookieMetadataFactory - ->createPublicCookieMetadata(); - $cookieMetadata->setDomain($this->sessionConfig->getCookieDomain()); - $cookieMetadata->setPath($this->sessionConfig->getCookiePath()); - $lifetime = $this->sessionConfig->getCookieLifetime(); - if ($lifetime !== 0) { - $cookieMetadata->setDuration($lifetime); - } - - $this->cookieFormKey->set( - $formKey, - $cookieMetadata - ); - } -} diff --git a/app/code/Magento/PageCache/Plugin/RegisterFormKeyFromCookie.php b/app/code/Magento/PageCache/Plugin/RegisterFormKeyFromCookie.php new file mode 100644 index 0000000000000..6cdc500aaf33c --- /dev/null +++ b/app/code/Magento/PageCache/Plugin/RegisterFormKeyFromCookie.php @@ -0,0 +1,107 @@ +cookieFormKey = $cacheFormKey; + $this->escaper = $escaper; + $this->formKey = $formKey; + $this->cookieMetadataFactory = $cookieMetadataFactory; + $this->sessionConfig = $sessionConfig; + } + + /** + * Set form key from the cookie. + * + * @return void + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function beforeDispatch(): void + { + if ($this->cookieFormKey->get()) { + $this->updateCookieFormKey($this->cookieFormKey->get()); + + $this->formKey->set( + $this->escaper->escapeHtml($this->cookieFormKey->get()) + ); + } + } + + /** + * @param string $formKey + * @return void + */ + private function updateCookieFormKey(string $formKey): void + { + $cookieMetadata = $this->cookieMetadataFactory + ->createPublicCookieMetadata(); + $cookieMetadata->setDomain($this->sessionConfig->getCookieDomain()); + $cookieMetadata->setPath($this->sessionConfig->getCookiePath()); + $lifetime = $this->sessionConfig->getCookieLifetime(); + if ($lifetime !== 0) { + $cookieMetadata->setDuration($lifetime); + } + + $this->cookieFormKey->set( + $formKey, + $cookieMetadata + ); + } +} diff --git a/app/code/Magento/PageCache/Test/Unit/Observer/RegisterFormKeyFromCookieTest.php b/app/code/Magento/PageCache/Test/Unit/Observer/RegisterFormKeyFromCookieTest.php deleted file mode 100644 index 7bdea2e427c0b..0000000000000 --- a/app/code/Magento/PageCache/Test/Unit/Observer/RegisterFormKeyFromCookieTest.php +++ /dev/null @@ -1,169 +0,0 @@ -cookieFormKey = $this->getMockBuilder( - \Magento\Framework\App\PageCache\FormKey::class - ) - ->disableOriginalConstructor() - ->getMock(); - $this->escaper = $this->getMockBuilder( - \Magento\Framework\Escaper::class - ) - ->disableOriginalConstructor() - ->getMock(); - $this->sessionFormKey = $this->getMockBuilder( - \Magento\Framework\Data\Form\FormKey::class - ) - ->disableOriginalConstructor() - ->getMock(); - $this->cookieMetadataFactory = $this->getMockBuilder( - \Magento\Framework\Stdlib\Cookie\CookieMetadataFactory::class - ) - ->disableOriginalConstructor() - ->getMock(); - $this->sessionConfig = $this->createMock( - \Magento\Framework\Session\Config\ConfigInterface::class - ); - - $this->observerMock = $this->createMock(\Magento\Framework\Event\Observer::class); - - $this->observer = new RegisterFormKeyFromCookie( - $this->cookieFormKey, - $this->escaper, - $this->sessionFormKey, - $this->cookieMetadataFactory, - $this->sessionConfig - ); - } - - public function testExecuteNoCookie() - { - $this->cookieFormKey->expects(static::once()) - ->method('get') - ->willReturn(null); - $this->cookieFormKey->expects(static::never()) - ->method('set'); - $this->sessionFormKey->expects(static::never()) - ->method('set'); - - $this->observer->execute($this->observerMock); - } - - public function testExecute() - { - $formKey = 'form_key'; - $escapedFormKey = 'escaped_form_key'; - $cookieDomain = 'example.com'; - $cookiePath = '/'; - $cookieLifetime = 3600; - - $cookieMetadata = $this->getMockBuilder( - \Magento\Framework\Stdlib\Cookie\PublicCookieMetadata::class - ) - ->disableOriginalConstructor() - ->getMock(); - - $this->cookieFormKey->expects(static::any()) - ->method('get') - ->willReturn($formKey); - $this->cookieMetadataFactory->expects(static::once()) - ->method('createPublicCookieMetadata') - ->willReturn( - $cookieMetadata - ); - - $this->sessionConfig->expects(static::once()) - ->method('getCookieDomain') - ->willReturn( - $cookieDomain - ); - $cookieMetadata->expects(static::once()) - ->method('setDomain') - ->with( - $cookieDomain - ); - $this->sessionConfig->expects(static::once()) - ->method('getCookiePath') - ->willReturn( - $cookiePath - ); - $cookieMetadata->expects(static::once()) - ->method('setPath') - ->with( - $cookiePath - ); - $this->sessionConfig->expects(static::once()) - ->method('getCookieLifetime') - ->willReturn( - $cookieLifetime - ); - $cookieMetadata->expects(static::once()) - ->method('setDuration') - ->with( - $cookieLifetime - ); - - $this->cookieFormKey->expects(static::once()) - ->method('set') - ->with( - $formKey, - $cookieMetadata - ); - - $this->escaper->expects(static::once()) - ->method('escapeHtml') - ->with($formKey) - ->willReturn($escapedFormKey); - - $this->sessionFormKey->expects(static::once()) - ->method('set') - ->with($escapedFormKey); - - $this->observer->execute($this->observerMock); - } -} diff --git a/app/code/Magento/PageCache/etc/di.xml b/app/code/Magento/PageCache/etc/di.xml index 2a9abbb860805..adf9526fc1108 100644 --- a/app/code/Magento/PageCache/etc/di.xml +++ b/app/code/Magento/PageCache/etc/di.xml @@ -37,6 +37,9 @@ Magento\Framework\View\Layout\LayoutCacheKeyInterface + + + diff --git a/app/code/Magento/PageCache/etc/frontend/events.xml b/app/code/Magento/PageCache/etc/frontend/events.xml deleted file mode 100644 index ce2c1c823e835..0000000000000 --- a/app/code/Magento/PageCache/etc/frontend/events.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - diff --git a/app/code/Magento/Wishlist/Test/Mftf/Test/StorefrontDeletePersistedWishlistTest.xml b/app/code/Magento/Wishlist/Test/Mftf/Test/StorefrontDeletePersistedWishlistTest.xml index 3ddb9d87073f3..cbed63db36d49 100644 --- a/app/code/Magento/Wishlist/Test/Mftf/Test/StorefrontDeletePersistedWishlistTest.xml +++ b/app/code/Magento/Wishlist/Test/Mftf/Test/StorefrontDeletePersistedWishlistTest.xml @@ -14,6 +14,8 @@ <description value="Customer should be able to delete a persistent wishlist"/> <group value="wishlist"/> + <!-- MQE-1145 --> + <group value="skip"/> </annotations> <before> <createData stepKey="category" entity="SimpleSubCategory"/> diff --git a/app/etc/di.xml b/app/etc/di.xml index 1f5507deb0519..6a4a6d16b5568 100755 --- a/app/etc/di.xml +++ b/app/etc/di.xml @@ -204,6 +204,8 @@ <preference for="Magento\Framework\MessageQueue\ExchangeFactoryInterface" type="Magento\Framework\MessageQueue\ExchangeFactory" /> <preference for="Magento\Framework\MessageQueue\Bulk\ExchangeFactoryInterface" type="Magento\Framework\MessageQueue\Bulk\ExchangeFactory" /> <preference for="Magento\Framework\MessageQueue\QueueFactoryInterface" type="Magento\Framework\MessageQueue\QueueFactory" /> + <preference for="Magento\Framework\App\Request\ValidatorInterface" + type="Magento\Framework\App\Request\CsrfValidator" /> <type name="Magento\Framework\Model\ResourceModel\Db\TransactionManager" shared="false" /> <type name="Magento\Framework\Acl\Data\Cache"> <arguments> diff --git a/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php b/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php index 83f86b264737c..0a8db19afe971 100644 --- a/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php +++ b/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php @@ -64,7 +64,7 @@ public function __construct(CurlTransport $transport, Customer $customer) protected function authorize(Customer $customer) { $url = $_ENV['app_frontend_url'] . 'customer/account/login/'; - $this->transport->write($url); + $this->transport->write($url, [], CurlInterface::GET); $this->read(); $url = $_ENV['app_frontend_url'] . 'customer/account/loginPost/'; $data = [ diff --git a/dev/tests/integration/framework/Magento/TestFramework/TestCase/AbstractController.php b/dev/tests/integration/framework/Magento/TestFramework/TestCase/AbstractController.php index 9920f90193f69..1bf6d471fc40a 100644 --- a/dev/tests/integration/framework/Magento/TestFramework/TestCase/AbstractController.php +++ b/dev/tests/integration/framework/Magento/TestFramework/TestCase/AbstractController.php @@ -9,9 +9,11 @@ */ namespace Magento\TestFramework\TestCase; +use Magento\Framework\Data\Form\FormKey; use Magento\Framework\Stdlib\CookieManagerInterface; use Magento\Framework\View\Element\Message\InterpretationStrategyInterface; use Magento\Theme\Controller\Result\MessagePlugin; +use Magento\Framework\App\Request\Http as HttpRequest; /** * @SuppressWarnings(PHPMD.NumberOfChildren) @@ -96,7 +98,16 @@ protected function assertPostConditions() */ public function dispatch($uri) { - $this->getRequest()->setRequestUri($uri); + /** @var HttpRequest $request */ + $request = $this->getRequest(); + $request->setRequestUri($uri); + if ($request->isPost() + && !array_key_exists('form_key', $request->getPost()) + ) { + /** @var FormKey $formKey */ + $formKey = $this->_objectManager->get(FormKey::class); + $request->setPostValue('form_key', $formKey->getFormKey()); + } $this->_getBootstrap()->runApp(); } diff --git a/dev/tests/integration/testsuite/Magento/Backend/App/Request/BackendValidatorTest.php b/dev/tests/integration/testsuite/Magento/Backend/App/Request/BackendValidatorTest.php new file mode 100644 index 0000000000000..21ffddf851ac4 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Backend/App/Request/BackendValidatorTest.php @@ -0,0 +1,433 @@ +<?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\Backend\App\Action\Context; +use Magento\Backend\Model\Auth; +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\Request\InvalidRequestException; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\ResponseInterface; +use Magento\Framework\Controller\ResultInterface; +use Magento\Framework\Data\Form\FormKey; +use Magento\Framework\Exception\NotFoundException; +use Magento\Framework\Phrase; +use Magento\TestFramework\Request; +use Magento\TestFramework\Response; +use PHPUnit\Framework\TestCase; +use Magento\TestFramework\Helper\Bootstrap; +use Magento\TestFramework\Bootstrap as TestBootstrap; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\App\Response\Http as HttpResponse; +use Zend\Stdlib\Parameters; +use Magento\Backend\Model\UrlInterface as BackendUrl; +use Magento\Framework\App\Response\HttpFactory as HttpResponseFactory; + +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ +class BackendValidatorTest extends TestCase +{ + private const AWARE_VALIDATION_PARAM = 'test_param'; + + private const AWARE_LOCATION_VALUE = 'test1'; + + private const CSRF_AWARE_MESSAGE = 'csrf_aware'; + + /** + * @var ActionInterface + */ + private $mockUnawareAction; + + /** + * @var AbstractAction + */ + private $mockAwareAction; + + /** + * @var BackendValidator + */ + private $validator; + + /** + * @var Request + */ + private $request; + + /** + * @var FormKey + */ + private $formKey; + + /** + * @var BackendUrl + */ + private $url; + + /** + * @var Auth + */ + private $auth; + + /** + * @var CsrfAwareActionInterface + */ + private $mockCsrfAwareAction; + + /** + * @var HttpResponseFactory + */ + private $httpResponseFactory; + + /** + * @return ActionInterface + */ + private function createUnawareAction(): ActionInterface + { + return new class implements ActionInterface { + /** + * @inheritDoc + */ + public function execute() + { + throw new NotFoundException(new Phrase('Not implemented')); + } + }; + } + + /** + * @return AbstractAction + */ + private function createAwareAction(): AbstractAction + { + $l = self::AWARE_LOCATION_VALUE; + $p = self::AWARE_VALIDATION_PARAM; + + return new class($l, $p) extends AbstractAction{ + + /** + * @var string + */ + private $locationValue; + + /** + * @var string + */ + private $param; + + /** + * @param string $locationValue + * @param string $param + */ + public function __construct( + string $locationValue, + string $param + ) { + parent::__construct( + Bootstrap::getObjectManager()->get(Context::class) + ); + $this->locationValue= $locationValue; + $this->param = $param; + } + + /** + * @inheritDoc + */ + public function execute() + { + throw new NotFoundException(new Phrase('Not implemented')); + } + + /** + * @inheritDoc + */ + public function _processUrlKeys() + { + if ($this->_request->getParam($this->param)) { + return true; + } else { + /** @var Response $response */ + $response = $this->_response; + $response->setHeader('Location', $this->locationValue); + + return false; + } + } + }; + } + + /** + * @return CsrfAwareActionInterface + */ + private function createCsrfAwareAction(): CsrfAwareActionInterface + { + $r = Bootstrap::getObjectManager() + ->get(ResponseInterface::class); + $m = self::CSRF_AWARE_MESSAGE; + + return new class ($r, $m) implements CsrfAwareActionInterface { + + /** + * @var ResponseInterface + */ + private $response; + + /** + * @var string + */ + private $message; + + /** + * @param ResponseInterface $response + * @param string $message + */ + public function __construct( + ResponseInterface $response, + string $message + ) { + $this->response = $response; + $this->message = $message; + } + + /** + * @inheritDoc + */ + public function execute() + { + return $this->response; + } + + /** + * @inheritDoc + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException { + return new InvalidRequestException( + $this->response, + [new Phrase($this->message)] + ); + } + + /** + * @inheritDoc + */ + public function validateForCsrf(RequestInterface $request): ?bool + { + return false; + } + + }; + } + + /** + * @inheritDoc + */ + protected function setUp() + { + $objectManager = Bootstrap::getObjectManager(); + $this->request = $objectManager->get(RequestInterface::class); + $this->validator = $objectManager->get(BackendValidator::class); + $this->mockUnawareAction = $this->createUnawareAction(); + $this->mockAwareAction = $this->createAwareAction(); + $this->formKey = $objectManager->get(FormKey::class); + $this->url = $objectManager->get(BackendUrl::class); + $this->auth = $objectManager->get(Auth::class); + $this->mockCsrfAwareAction = $this->createCsrfAwareAction(); + $this->httpResponseFactory = $objectManager->get( + HttpResponseFactory::class + ); + } + + /** + * @magentoConfigFixture admin/security/use_form_key 1 + * @magentoAppArea adminhtml + */ + public function testValidateWithValidKey() + { + $this->request->setMethod(HttpRequest::METHOD_GET); + $this->auth->login( + TestBootstrap::ADMIN_NAME, + TestBootstrap::ADMIN_PASSWORD + ); + $this->request->setParams([ + BackendUrl::SECRET_KEY_PARAM_NAME => $this->url->getSecretKey(), + ]); + + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @expectedException \Magento\Framework\App\Request\InvalidRequestException + * + * @magentoConfigFixture admin/security/use_form_key 1 + * @magentoAppArea adminhtml + */ + public function testValidateWithInvalidKey() + { + $invalidKey = $this->url->getSecretKey() .'Invalid'; + $this->request->setParams([ + BackendUrl::SECRET_KEY_PARAM_NAME => $invalidKey, + ]); + $this->request->setMethod(HttpRequest::METHOD_GET); + $this->auth->login( + TestBootstrap::ADMIN_NAME, + TestBootstrap::ADMIN_PASSWORD + ); + + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @expectedException \Magento\Framework\App\Request\InvalidRequestException + * + * @magentoConfigFixture admin/security/use_form_key 0 + * @magentoAppArea adminhtml + */ + public function testValidateWithInvalidFormKey() + { + $this->request->setPost( + new Parameters(['form_key' => $this->formKey->getFormKey() .'1']) + ); + $this->request->setMethod(HttpRequest::METHOD_POST); + + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @magentoConfigFixture admin/security/use_form_key 0 + * @magentoAppArea adminhtml + */ + public function testValidateInvalidWithAwareAction() + { + $this->request->setParams([self::AWARE_VALIDATION_PARAM => '']); + + /** @var InvalidRequestException|null $caught */ + $caught = null; + try { + $this->validator->validate( + $this->request, + $this->mockAwareAction + ); + } catch (InvalidRequestException $exception) { + $caught = $exception; + } + + $this->assertNotNull($caught); + /** @var Response $response */ + $response = $caught->getReplaceResult(); + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals( + self::AWARE_LOCATION_VALUE, + $response->getHeader('Location')->getFieldValue() + ); + $this->assertNull($caught->getMessages()); + } + + /** + * @magentoAppArea adminhtml + */ + public function testValidateValidWithAwareAction() + { + $this->request->setParams( + [self::AWARE_VALIDATION_PARAM => '1'] + ); + + $this->validator->validate( + $this->request, + $this->mockAwareAction + ); + } + + /** + * @magentoConfigFixture admin/security/use_form_key 1 + * @magentoAppArea adminhtml + */ + public function testValidateWithCsrfAwareAction() + { + //Setting up request that would be valid for default validation. + $this->request->setMethod(HttpRequest::METHOD_GET); + $this->auth->login( + TestBootstrap::ADMIN_NAME, + TestBootstrap::ADMIN_PASSWORD + ); + $this->request->setParams([ + BackendUrl::SECRET_KEY_PARAM_NAME => $this->url->getSecretKey(), + ]); + + /** @var InvalidRequestException|null $caught */ + $caught = null; + try { + $this->validator->validate( + $this->request, + $this->mockCsrfAwareAction + ); + } catch (InvalidRequestException $exception) { + $caught = $exception; + } + + //Checking that custom validation was called and invalidated + //valid request. + $this->assertNotNull($caught); + $this->assertCount(1, $caught->getMessages()); + $this->assertEquals( + self::CSRF_AWARE_MESSAGE, + $caught->getMessages()[0]->getText() + ); + } + + public function testInvalidAjaxRequest() + { + //Setting up AJAX request with invalid secret key. + $this->request->setMethod(HttpRequest::METHOD_GET); + $this->auth->login( + TestBootstrap::ADMIN_NAME, + TestBootstrap::ADMIN_PASSWORD + ); + $this->request->setParams([ + BackendUrl::SECRET_KEY_PARAM_NAME => 'invalid', + 'isAjax' => '1' + ]); + + /** @var InvalidRequestException|null $caught */ + $caught = null; + try { + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } catch (InvalidRequestException $exception) { + $caught = $exception; + } + + $this->assertNotNull($caught); + $this->assertInstanceOf( + ResultInterface::class, + $caught->getReplaceResult() + ); + /** @var ResultInterface $result */ + $result = $caught->getReplaceResult(); + /** @var HttpResponse $response */ + $response = $this->httpResponseFactory->create(); + $result->renderResult($response); + $this->assertEmpty($response->getBody()); + $this->assertEquals(401, $response->getHttpResponseCode()); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php b/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php index eca5cf79c0664..1cbdbd128bbf4 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php +++ b/dev/tests/integration/testsuite/Magento/Customer/Controller/AccountTest.php @@ -219,6 +219,7 @@ public function testConfirmActionAlreadyActive() public function testNoFormKeyCreatePostAction() { $this->fillRequestWithAccountData('test1@email.com'); + $this->getRequest()->setPostValue('form_key', null); $this->dispatch('customer/account/createPost'); $this->assertNull($this->getCustomerByEmail('test1@email.com')); diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/Request/CsrfValidatorTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/Request/CsrfValidatorTest.php new file mode 100644 index 0000000000000..9246be52f41bf --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/Request/CsrfValidatorTest.php @@ -0,0 +1,269 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\App\Request; + +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\Controller\Result\Redirect; +use Magento\Framework\Controller\Result\RedirectFactory; +use Magento\Framework\Data\Form\FormKey; +use Magento\Framework\Exception\NotFoundException; +use Magento\Framework\Phrase; +use PHPUnit\Framework\TestCase; +use Magento\TestFramework\Helper\Bootstrap; +use Magento\Framework\App\Request\Http as HttpRequest; +use Zend\Stdlib\Parameters; +use Magento\Framework\App\Response\Http as HttpResponse; +use Magento\Framework\App\Response\HttpFactory as HttpResponseFactory; + +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ +class CsrfValidatorTest extends TestCase +{ + private const AWARE_URL = 'test/1'; + + private const AWARE_VALIDATION_PARAM = 'test_param'; + + private const AWARE_MESSAGE = 'custom validation failed'; + + /** + * @var ActionInterface + */ + private $mockUnawareAction; + + /** + * @var ActionInterface + */ + private $mockAwareAction; + + /** + * @var CsrfValidator + */ + private $validator; + + /** + * @var HttpRequest + */ + private $request; + + /** + * @var FormKey + */ + private $formKey; + + /** + * @var HttpResponseFactory + */ + private $httpResponseFactory; + + /** + * @return ActionInterface + */ + private function createUnawareAction(): ActionInterface + { + return new class implements ActionInterface { + /** + * @inheritDoc + */ + public function execute() + { + throw new NotFoundException(new Phrase('Not implemented')); + } + }; + } + + /** + * @return ActionInterface + */ + private function createAwareAction(): ActionInterface + { + $u = self::AWARE_URL; + $m = self::AWARE_MESSAGE; + $p = self::AWARE_VALIDATION_PARAM; + + return new class($u, $m, $p) implements CsrfAwareActionInterface { + /** + * @var string + */ + private $url; + + /** + * @var string + */ + private $message; + + /** + * @var string + */ + private $param; + + /** + * @param string $url + * @param string $message + * @param string $param + */ + public function __construct( + string $url, + string $message, + string $param + ) { + $this->url = $url; + $this->message = $message; + $this->param = $param; + } + + /** + * @inheritDoc + */ + public function execute() + { + throw new NotFoundException(new Phrase('Not implemented')); + } + + /** + * @inheritDoc + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException { + /** @var RedirectFactory $redirectFactory */ + $redirectFactory = Bootstrap::getObjectManager() + ->get(RedirectFactory::class); + $redirect = $redirectFactory->create(); + $redirect->setUrl($this->url); + + return new InvalidRequestException( + $redirect, + [new Phrase($this->message)] + ); + } + + /** + * @inheritDoc + */ + public function validateForCsrf(RequestInterface $request): ?bool + { + return (bool)$request->getParam($this->param); + } + }; + } + + /** + * @inheritDoc + */ + protected function setUp() + { + $objectManager = Bootstrap::getObjectManager(); + $this->request = $objectManager->get(HttpRequest::class); + $this->validator = $objectManager->get(CsrfValidator::class); + $this->mockUnawareAction = $this->createUnawareAction(); + $this->mockAwareAction = $this->createAwareAction(); + $this->formKey = $objectManager->get(FormKey::class); + $this->httpResponseFactory = $objectManager->get( + HttpResponseFactory::class + ); + } + + /** + * @magentoAppArea global + */ + public function testValidateInWrongArea() + { + $this->request->setMethod(HttpRequest::METHOD_POST); + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @magentoAppArea frontend + */ + public function testValidateWithValidKey() + { + $this->request->setPost( + new Parameters(['form_key' => $this->formKey->getFormKey()]) + ); + $this->request->setMethod(HttpRequest::METHOD_POST); + + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @expectedException \Magento\Framework\App\Request\InvalidRequestException + * @magentoAppArea adminhtml + */ + public function testValidateWithInvalidKey() + { + $this->request->setPost( + new Parameters(['form_key' => $this->formKey->getFormKey() .'1']) + ); + $this->request->setMethod(HttpRequest::METHOD_POST); + + $this->validator->validate( + $this->request, + $this->mockUnawareAction + ); + } + + /** + * @magentoAppArea frontend + */ + public function testValidateInvalidWithAwareAction() + { + $this->request->setMethod(HttpRequest::METHOD_POST); + + /** @var InvalidRequestException|null $caught */ + $caught = null; + try { + $this->validator->validate( + $this->request, + $this->mockAwareAction + ); + } catch (InvalidRequestException $exception) { + $caught = $exception; + } + + $this->assertNotNull($caught); + $this->assertInstanceOf(Redirect::class, $caught->getReplaceResult()); + /** @var HttpResponse $response */ + $response = $this->httpResponseFactory->create(); + $caught->getReplaceResult()->renderResult($response); + $this->assertContains( + self::AWARE_URL, + $response->getHeaders()->toString() + ); + $this->assertCount(1, $caught->getMessages()); + $this->assertEquals( + self::AWARE_MESSAGE, + $caught->getMessages()[0]->getText() + ); + } + + /** + * @magentoAppArea frontend + */ + public function testValidateValidWithAwareAction() + { + $this->request->setMethod(HttpRequest::METHOD_POST); + $this->request->setPost( + new Parameters([self::AWARE_VALIDATION_PARAM => '1']) + ); + + $this->validator->validate( + $this->request, + $this->mockAwareAction + ); + } +} diff --git a/dev/tests/integration/testsuite/Magento/PageCache/Plugin/RegisterFormKeyFromCookieTest.php b/dev/tests/integration/testsuite/Magento/PageCache/Plugin/RegisterFormKeyFromCookieTest.php new file mode 100644 index 0000000000000..eb23b0b185a00 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/PageCache/Plugin/RegisterFormKeyFromCookieTest.php @@ -0,0 +1,68 @@ +<?php +/** + * + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\PageCache\Plugin; + +use Magento\Framework\App\FrontController; +use Magento\Framework\App\RequestInterface; +use PHPUnit\Framework\TestCase; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\Data\Form\FormKey\Validator as FormKeyValidator; +use Magento\TestFramework\Helper\Bootstrap; + +class RegisterFormKeyFromCookieTest extends TestCase +{ + /** + * @var HttpRequest + */ + private $request; + + /** + * @var FrontController + */ + private $frontController; + + /** + * @var FormKeyValidator + */ + private $formKeyValidator; + + /** + * @inheritDoc + */ + protected function setUp() + { + $objectManager = Bootstrap::getObjectManager(); + $this->request = $objectManager->get(RequestInterface::class); + $this->frontController = $objectManager->get( + FrontController::class + ); + $this->formKeyValidator = $objectManager->get(FormKeyValidator::class); + } + + /** + * @magentoAppArea frontend + */ + public function testTakenFromCookie() + { + if (!Bootstrap::canTestHeaders()) { + $this->markTestSkipped( + 'Can\'t test dispatch process without sending headers' + ); + } + $_SERVER['HTTP_HOST'] = 'localhost'; + $formKey = 'customFormKey'; + $_COOKIE['form_key'] = $formKey; + $this->request->setMethod(HttpRequest::METHOD_POST); + $this->request->setParam('form_key', $formKey); + $this->request->setRequestUri('core/index/index'); + $this->frontController->dispatch($this->request); + $this->assertTrue($this->formKeyValidator->validate($this->request)); + } +} diff --git a/lib/internal/Magento/Framework/App/CsrfAwareActionInterface.php b/lib/internal/Magento/Framework/App/CsrfAwareActionInterface.php new file mode 100644 index 0000000000000..8413b945a1141 --- /dev/null +++ b/lib/internal/Magento/Framework/App/CsrfAwareActionInterface.php @@ -0,0 +1,39 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\App; + +use Magento\Framework\App\Request\InvalidRequestException; + +/** + * Action that's aware of CSRF protection. + */ +interface CsrfAwareActionInterface extends ActionInterface +{ + /** + * Create exception in case CSRF validation failed. + * Return null if default exception will suffice. + * + * @param RequestInterface $request + * + * @return InvalidRequestException|null + */ + public function createCsrfValidationException( + RequestInterface $request + ): ?InvalidRequestException; + + /** + * Perform custom request validation. + * Return null if default validation is needed. + * + * @param RequestInterface $request + * + * @return bool|null + */ + public function validateForCsrf(RequestInterface $request): ?bool; +} diff --git a/lib/internal/Magento/Framework/App/FrontController.php b/lib/internal/Magento/Framework/App/FrontController.php index 02b390289c9a9..03d6ad7ab3f02 100644 --- a/lib/internal/Magento/Framework/App/FrontController.php +++ b/lib/internal/Magento/Framework/App/FrontController.php @@ -7,6 +7,16 @@ */ namespace Magento\Framework\App; +use Magento\Framework\App\Request\InvalidRequestException; +use Magento\Framework\Controller\ResultInterface; +use Magento\Framework\App\Request\ValidatorInterface as RequestValidator; +use Magento\Framework\Exception\NotFoundException; +use Magento\Framework\Message\ManagerInterface as MessageManager; +use Magento\Framework\App\Action\AbstractAction; + +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class FrontController implements FrontControllerInterface { /** @@ -15,27 +25,45 @@ class FrontController implements FrontControllerInterface protected $_routerList; /** - * @var \Magento\Framework\App\ResponseInterface + * @var ResponseInterface */ protected $response; + /** + * @var RequestValidator + */ + private $requestValidator; + + /** + * @var MessageManager + */ + private $messages; + /** * @param RouterListInterface $routerList - * @param \Magento\Framework\App\ResponseInterface $response + * @param ResponseInterface $response + * @param RequestValidator|null $requestValidator + * @param MessageManager|null $messageManager */ public function __construct( RouterListInterface $routerList, - \Magento\Framework\App\ResponseInterface $response + ResponseInterface $response, + ?RequestValidator $requestValidator = null, + ?MessageManager $messageManager = null ) { $this->_routerList = $routerList; $this->response = $response; + $this->requestValidator = $requestValidator + ?? ObjectManager::getInstance()->get(RequestValidator::class); + $this->messages = $messageManager + ?? ObjectManager::getInstance()->get(MessageManager::class); } /** * Perform action and generate response * * @param RequestInterface $request - * @return ResponseInterface|\Magento\Framework\Controller\ResultInterface + * @return ResponseInterface|ResultInterface * @throws \LogicException */ public function dispatch(RequestInterface $request) @@ -49,13 +77,10 @@ public function dispatch(RequestInterface $request) try { $actionInstance = $router->match($request); if ($actionInstance) { - $request->setDispatched(true); - $this->response->setNoCacheHeaders(); - if ($actionInstance instanceof \Magento\Framework\App\Action\AbstractAction) { - $result = $actionInstance->dispatch($request); - } else { - $result = $actionInstance->execute(); - } + $result = $this->processRequest( + $request, + $actionInstance + ); break; } } catch (\Magento\Framework\Exception\NotFoundException $e) { @@ -72,4 +97,42 @@ public function dispatch(RequestInterface $request) } return $result; } + + /** + * @param RequestInterface $request + * @param ActionInterface $actionInstance + * @throws NotFoundException + * + * @return ResponseInterface|ResultInterface + */ + private function processRequest( + RequestInterface $request, + ActionInterface $actionInstance + ) { + $request->setDispatched(true); + $this->response->setNoCacheHeaders(); + //Validating request. + try { + $this->requestValidator->validate( + $request, + $actionInstance + ); + + if ($actionInstance instanceof AbstractAction) { + $result = $actionInstance->dispatch($request); + } else { + $result = $actionInstance->execute(); + } + } catch (InvalidRequestException $exception) { + //Validation failed - processing validation results. + $result = $exception->getReplaceResult(); + if ($messages = $exception->getMessages()) { + foreach ($messages as $message) { + $this->messages->addErrorMessage($message); + } + } + } + + return $result; + } } diff --git a/lib/internal/Magento/Framework/App/Request/CsrfValidator.php b/lib/internal/Magento/Framework/App/Request/CsrfValidator.php new file mode 100644 index 0000000000000..c930fc920907c --- /dev/null +++ b/lib/internal/Magento/Framework/App/Request/CsrfValidator.php @@ -0,0 +1,132 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\App\Request; + +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\Area; +use Magento\Framework\App\CsrfAwareActionInterface; +use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\State as AppState; +use Magento\Framework\Data\Form\FormKey\Validator as FormKeyValidator; +use Magento\Framework\Controller\Result\RedirectFactory; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Phrase; + +/** + * Validate request for being CSRF protected. + */ +class CsrfValidator implements ValidatorInterface +{ + /** + * @var FormKeyValidator + */ + private $formKeyValidator; + + /** + * @var RedirectFactory + */ + private $redirectFactory; + + /** + * @var AppState + */ + private $appState; + + /** + * @param FormKeyValidator $formKeyValidator + * @param RedirectFactory $redirectFactory + * @param AppState $appState + */ + public function __construct( + FormKeyValidator $formKeyValidator, + RedirectFactory $redirectFactory, + AppState $appState + ) { + $this->formKeyValidator = $formKeyValidator; + $this->redirectFactory = $redirectFactory; + $this->appState = $appState; + } + + /** + * @param HttpRequest $request + * @param ActionInterface $action + * + * @return bool + */ + private function validateRequest( + HttpRequest $request, + ActionInterface $action + ): bool { + $valid = null; + if ($action instanceof CsrfAwareActionInterface) { + $valid = $action->validateForCsrf($request); + } + if ($valid === null) { + $valid = !$request->isPost() + || $request->isAjax() + || $this->formKeyValidator->validate($request); + } + + return $valid; + } + + /** + * @param HttpRequest $request + * @param ActionInterface $action + * + * @return InvalidRequestException + */ + private function createException( + HttpRequest $request, + ActionInterface $action + ): InvalidRequestException { + $exception = null; + if ($action instanceof CsrfAwareActionInterface) { + $exception = $action->createCsrfValidationException($request); + } + if (!$exception) { + $response = $this->redirectFactory->create() + ->setRefererOrBaseUrl() + ->setHttpResponseCode(302); + $messages = [ + new Phrase('Invalid Form Key. Please refresh the page.'), + ]; + $exception = new InvalidRequestException($response, $messages); + } + + return $exception; + } + + /** + * @inheritDoc + */ + public function validate( + RequestInterface $request, + ActionInterface $action + ): void { + try { + $areaCode = $this->appState->getAreaCode(); + } catch (LocalizedException $exception) { + $areaCode = null; + } + if ($request instanceof HttpRequest + && in_array( + $areaCode, + [Area::AREA_FRONTEND, Area::AREA_ADMINHTML], + true + ) + ) { + $valid = $this->validateRequest($request, $action); + if (!$valid) { + throw $this->createException($request, $action); + } + } + } +} diff --git a/lib/internal/Magento/Framework/App/Request/InvalidRequestException.php b/lib/internal/Magento/Framework/App/Request/InvalidRequestException.php new file mode 100644 index 0000000000000..3d408b0050686 --- /dev/null +++ b/lib/internal/Magento/Framework/App/Request/InvalidRequestException.php @@ -0,0 +1,60 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\App\Request; + +use Magento\Framework\App\ResponseInterface; +use Magento\Framework\Controller\ResultInterface; +use Magento\Framework\Exception\RuntimeException; +use Magento\Framework\Phrase; + +/** + * Received request is invalid. + */ +class InvalidRequestException extends RuntimeException +{ + /** + * @var ResponseInterface|ResultInterface + */ + private $replaceResult; + + /** + * @var Phrase[]|null + */ + private $messages; + + /** + * @param ResponseInterface|ResultInterface $replaceResult Use this result + * instead of calling action instance. + * @param Phrase[]|null $messages Messages to show to client + * as error messages. + */ + public function __construct($replaceResult, ?array $messages = null) + { + parent::__construct(new Phrase('Invalid request received')); + + $this->replaceResult = $replaceResult; + $this->messages = $messages; + } + + /** + * @return ResponseInterface|ResultInterface + */ + public function getReplaceResult() + { + return $this->replaceResult; + } + + /** + * @return Phrase[]|null + */ + public function getMessages(): ?array + { + return $this->messages; + } +} diff --git a/lib/internal/Magento/Framework/App/Request/ValidatorInterface.php b/lib/internal/Magento/Framework/App/Request/ValidatorInterface.php new file mode 100644 index 0000000000000..f4b1503ac5cb6 --- /dev/null +++ b/lib/internal/Magento/Framework/App/Request/ValidatorInterface.php @@ -0,0 +1,32 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\App\Request; + +use Magento\Framework\App\ActionInterface; +use Magento\Framework\App\RequestInterface; + +/** + * Validate interface before giving passing it to an ActionInterface. + */ +interface ValidatorInterface +{ + /** + * Validate request and throw the exception if it's invalid. + * + * @param RequestInterface $request + * @param ActionInterface $action + * @throws InvalidRequestException If request was invalid. + * + * @return void + */ + public function validate( + RequestInterface $request, + ActionInterface $action + ): void; +} diff --git a/lib/web/mage/common.js b/lib/web/mage/common.js index a333277d9e3cd..4742c77ea8627 100644 --- a/lib/web/mage/common.js +++ b/lib/web/mage/common.js @@ -11,4 +11,24 @@ define([ /* Form with auto submit feature */ $('form[data-auto-submit="true"]').submit(); + + //Add form keys. + $(document).on( + 'submit', + 'form', + function (e) { + var formKeyElement, + form = $(e.target), + formKey = $('input[name="form_key"]').val(); + + if (formKey && !form.find('input[name="form_key"]').length) { + formKeyElement = document.createElement('input'); + formKeyElement.setAttribute('type', 'hidden'); + formKeyElement.setAttribute('name', 'form_key'); + formKeyElement.setAttribute('value', formKey); + formKeyElement.setAttribute('auto-added-form-key', '1'); + form.get(0).appendChild(formKeyElement); + } + } + ); });