diff --git a/source/Application/Controller/OrderController.php b/source/Application/Controller/OrderController.php index 6e8f9ee311..2ac9033c34 100644 --- a/source/Application/Controller/OrderController.php +++ b/source/Application/Controller/OrderController.php @@ -7,9 +7,12 @@ namespace OxidEsales\EshopCommunity\Application\Controller; +use OxidEsales\Eshop\Application\Model\Order; use OxidEsales\Eshop\Core\Registry; use OxidEsales\Eshop\Core\UtilsObject; use OxidEsales\Eshop\Application\Model\BasketContentMarkGenerator; +use OxidEsales\EshopCommunity\Core\Di\ContainerFacade; +use Psr\Log\LoggerInterface; /** * Order manager. Arranges user ordering data, checks/validates @@ -114,7 +117,7 @@ class OrderController extends \OxidEsales\Eshop\Application\Controller\FrontendC public function init() { // disabling performance control variable - \OxidEsales\Eshop\Core\Registry::getConfig()->setConfigParam('bl_perfCalcVatOnlyForBasketOrder', false); + Registry::getConfig()->setConfigParam('bl_perfCalcVatOnlyForBasketOrder', false); // recalc basket cause of payment stuff if ($oBasket = $this->getBasket()) { @@ -138,10 +141,10 @@ public function render() { if ($this->getIsOrderStep()) { $oBasket = $this->getBasket(); - $myConfig = \OxidEsales\Eshop\Core\Registry::getConfig(); + $myConfig = Registry::getConfig(); + $session = Registry::getSession(); if ($myConfig->getConfigParam('blPsBasketReservationEnabled')) { - $session = \OxidEsales\Eshop\Core\Registry::getSession(); $session->getBasketReservations()->renewExpiration(); if (!$oBasket || ($oBasket && !$oBasket->getProductsCount())) { Registry::getUtils()->redirect($myConfig->getShopHomeUrl() . 'cl=basket', true, 302); @@ -163,11 +166,12 @@ public function render() } } + $this->_aViewData['basketSummaryHash'] = $this->getBasketSummaryHash(); parent::render(); // reload blocker - if (!Registry::getSession()->getVariable('sess_challenge')) { - Registry::getSession()->setVariable('sess_challenge', $this->getUtilsObjectInstance()->generateUID()); + if (!$session->getVariable('sess_challenge')) { + $session->setVariable('sess_challenge', $this->getUtilsObjectInstance()->generateUID()); } return $this->_sThisTemplate; @@ -186,44 +190,52 @@ public function render() */ public function execute() { - $session = \OxidEsales\Eshop\Core\Registry::getSession(); + $session = Registry::getSession(); if (!$session->checkSessionChallenge()) { - return; + return null; } if (!$this->validateTermsAndConditions()) { $this->_blConfirmAGBError = 1; - return; + return null; } - // additional check if we really really have a user now - $oUser = $this->getUser(); - if (!$oUser) { + $user = $this->getUser(); + if (!$user) { return 'user'; } + $basket = $session->getBasket(); + + $requestBasketSummaryHash = Registry::getRequest()->getRequestParameter('basketSummaryHash'); + if (!$requestBasketSummaryHash) { + $this->notifyIfBasketSummaryValidationIsNotPossible(); + } elseif ($requestBasketSummaryHash !== $this->getBasketSummaryHash()) { + $redirect = $basket->getProductsCount() === 0 ? 'basket' : 'order'; + $this->addBasketSummaryValidationError($redirect); + + return $redirect; + } - // get basket contents - $oBasket = $session->getBasket(); - if ($oBasket->getProductsCount()) { + if ($basket->getProductsCount()) { try { - $oOrder = oxNew(\OxidEsales\Eshop\Application\Model\Order::class); + $order = oxNew(Order::class); //finalizing ordering process (validating, storing order into DB, executing payment, setting status ...) - $iSuccess = $oOrder->finalizeOrder($oBasket, $oUser); + $success = $order->finalizeOrder($basket, $user); // performing special actions after user finishes order (assignment to special user groups) - $oUser->onOrderExecute($oBasket, $iSuccess); + $user->onOrderExecute($basket, $success); // proceeding to next view - return $this->getNextStep($iSuccess); - } catch (\OxidEsales\Eshop\Core\Exception\OutOfStockException $oEx) { - $oEx->setDestination('basket'); - Registry::getUtilsView()->addErrorToDisplay($oEx, false, true, 'basket'); - } catch (\OxidEsales\Eshop\Core\Exception\NoArticleException $oEx) { - Registry::getUtilsView()->addErrorToDisplay($oEx); - } catch (\OxidEsales\Eshop\Core\Exception\ArticleInputException $oEx) { - Registry::getUtilsView()->addErrorToDisplay($oEx); + return $this->getNextStep($success); + } catch (\OxidEsales\Eshop\Core\Exception\OutOfStockException $exception) { + $exception->setDestination('basket'); + Registry::getUtilsView()->addErrorToDisplay($exception, false, true, 'basket'); + } catch (\OxidEsales\Eshop\Core\Exception\NoArticleException $exception) { + Registry::getUtilsView()->addErrorToDisplay($exception); + } catch (\OxidEsales\Eshop\Core\Exception\ArticleInputException $exception) { + Registry::getUtilsView()->addErrorToDisplay($exception); } } } @@ -248,8 +260,8 @@ public function getPayment() if ( $sPaymentid && $oPayment->load($sPaymentid) && $oPayment->isValidPayment( - \OxidEsales\Eshop\Core\Registry::getSession()->getVariable('dynvalue'), - \OxidEsales\Eshop\Core\Registry::getConfig()->getShopId(), + Registry::getSession()->getVariable('dynvalue'), + Registry::getConfig()->getShopId(), $oUser, $oBasket->getPriceForPayment(), Registry::getSession()->getVariable('sShipSet') @@ -271,7 +283,7 @@ public function getBasket() { if ($this->_oBasket === null) { $this->_oBasket = false; - $session = \OxidEsales\Eshop\Core\Registry::getSession(); + $session = Registry::getSession(); if ($oBasket = $session->getBasket()) { $this->_oBasket = $oBasket; } @@ -333,7 +345,7 @@ public function getDelAddress() { if ($this->_oDelAddress === null) { $this->_oDelAddress = false; - $oOrder = oxNew(\OxidEsales\Eshop\Application\Model\Order::class); + $oOrder = oxNew(Order::class); $this->_oDelAddress = $oOrder->getDelAddressInfo(); } @@ -369,7 +381,7 @@ public function isConfirmAGBActive() { if ($this->_blConfirmAGB === null) { $this->_blConfirmAGB = false; - $this->_blConfirmAGB = \OxidEsales\Eshop\Core\Registry::getConfig()->getConfigParam('blConfirmAGB'); + $this->_blConfirmAGB = Registry::getConfig()->getConfigParam('blConfirmAGB'); } return $this->_blConfirmAGB; @@ -394,7 +406,7 @@ public function showOrderButtonOnTop() { if ($this->_blShowOrderButtonOnTop === null) { $this->_blShowOrderButtonOnTop = false; - $this->_blShowOrderButtonOnTop = \OxidEsales\Eshop\Core\Registry::getConfig()->getConfigParam('blShowOrderButtonOnTop'); + $this->_blShowOrderButtonOnTop = Registry::getConfig()->getConfigParam('blShowOrderButtonOnTop'); } return $this->_blShowOrderButtonOnTop; @@ -498,24 +510,24 @@ protected function getNextStep($iSuccess) //little trick with switch for multiple cases switch (true) { - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_MAILINGERROR): + case ($iSuccess === Order::ORDER_STATE_MAILINGERROR): $sNextStep = 'thankyou?mailerror=1'; break; - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_INVALIDDELADDRESSCHANGED): + case ($iSuccess === Order::ORDER_STATE_INVALIDDELADDRESSCHANGED): $sNextStep = 'order?iAddressError=1'; break; - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_BELOWMINPRICE): + case ($iSuccess === Order::ORDER_STATE_BELOWMINPRICE): $sNextStep = 'order'; break; - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_VOUCHERERROR): + case ($iSuccess === Order::ORDER_STATE_VOUCHERERROR): $sNextStep = 'basket'; break; - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_PAYMENTERROR): + case ($iSuccess === Order::ORDER_STATE_PAYMENTERROR): // no authentication, kick back to payment methods Registry::getSession()->setVariable('payerror', 2); $sNextStep = 'payment?payerror=2'; break; - case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_ORDEREXISTS): + case ($iSuccess === Order::ORDER_STATE_ORDEREXISTS): break; // reload blocker activ case (is_numeric($iSuccess) && $iSuccess > 3): Registry::getSession()->setVariable('payerror', $iSuccess); @@ -542,7 +554,7 @@ protected function getNextStep($iSuccess) protected function validateTermsAndConditions() { $blValid = true; - $oConfig = \OxidEsales\Eshop\Core\Registry::getConfig(); + $oConfig = Registry::getConfig(); if ($oConfig->getConfigParam('blConfirmAGB') && !Registry::getRequest()->getRequestEscapedParameter('ord_agb')) { $blValid = false; @@ -572,4 +584,30 @@ protected function getUtilsObjectInstance() { return Registry::getUtilsObject(); } + + private function getBasketSummaryHash(): string + { + return md5(json_encode($this->getBasket()->getBasketSummary())); + } + + private function notifyIfBasketSummaryValidationIsNotPossible(): void + { + ContainerFacade::get(LoggerInterface::class) + ->warning( + 'Pricing and payments verification can not be performed, ' . + 'the basketSummaryHash parameter was not sent with request data.' + ); + } + + private function addBasketSummaryValidationError(string $controller): void + { + Registry::getUtilsView() + ->addErrorToDisplay( + 'BASKET_ITEMS_CHANGED_ERROR', + false, + true, + '', + $controller + ); + } } diff --git a/source/Application/translations/de/lang.php b/source/Application/translations/de/lang.php index e1d6ac4854..7694ae6d91 100644 --- a/source/Application/translations/de/lang.php +++ b/source/Application/translations/de/lang.php @@ -54,6 +54,7 @@ 'BARGAIN' => 'Schnäppchen', 'BARGAIN_PRODUCTS' => 'Die besten Schnäppchen des Shops', 'BASKET_EMPTY' => 'Der Warenkorb ist leer.', +'BASKET_ITEMS_CHANGED_ERROR' => 'Die Warenkorbartikel wurden geändert.', 'BIC' => 'BIC', 'BILLING_ADDRESS' => 'Rechnungsadresse', 'BILLING_SHIPPING_SETTINGS' => 'Rechnungs- und Lieferadressen', diff --git a/source/Application/translations/en/lang.php b/source/Application/translations/en/lang.php index 355969e1a8..6af15e807d 100644 --- a/source/Application/translations/en/lang.php +++ b/source/Application/translations/en/lang.php @@ -54,6 +54,7 @@ 'BARGAIN' => 'Bargain', 'BARGAIN_PRODUCTS' => 'Bargain products', 'BASKET_EMPTY' => 'The shopping cart is empty.', +'BASKET_ITEMS_CHANGED_ERROR' => 'The shopping cart items have been changed.', 'BIC' => 'BIC', 'BILLING_ADDRESS' => 'Billing address', 'BILLING_SHIPPING_SETTINGS' => 'Billing and shipping addresses', diff --git a/tests/Codeception/Acceptance/CheckoutProcessCest.php b/tests/Codeception/Acceptance/CheckoutProcessCest.php index 93e8848a25..262471e2b9 100644 --- a/tests/Codeception/Acceptance/CheckoutProcessCest.php +++ b/tests/Codeception/Acceptance/CheckoutProcessCest.php @@ -825,6 +825,77 @@ public function checkOrderStepChangedAddress(AcceptanceTester $I): void $orderCheckout->submitOrderSuccessfully(); } + public function testWarningAboutBasketChanges(AcceptanceTester $I): void + { + $I->wantToTest('user sees warning before submitting an order if his cart was modified from another session'); + $user = Fixtures::get('existingUser'); + $product1 = Fixtures::get('product-1000'); + $product2 = Fixtures::get('product-1001'); + + $I->amGoingTo('add a product and go through till the last order step'); + $I->openShop() + ->loginUser( + $user['userLoginName'], + $user['userPassword'] + ); + $basket = new Basket($I); + $basket->addProductToBasket($product1['OXID'], 1); + $orderCheckout = $basket->openMiniBasket()->openCheckout()->goToNextStep(); + + $I->amGoingTo('add one more product to existing basket in another session (browser tab)'); + $I->openNewTab(); + $I->openShop(); + (new Basket($I))->addProductToBasket($product2['OXID'], 1); + + $I->amGoingTo('go back to the initial session (tab)'); + $I->closeTab(); + + $I->amGoingTo('try to submit an order and make sure I see the warning'); + $orderCheckout->submitOrder(); + $I->see($product1['OXTITLE_1']); + $I->see($product2['OXTITLE_1']); + $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR')); + + $I->amGoingTo('confirm that order can be submitted after the warning was shown'); + $orderCheckout->submitOrderSuccessfully(); + } + + public function testWarningAboutBasketChangesWithEmptyBasket(AcceptanceTester $I): void + { + $I->wantToTest('card-modified-warning-message behaviour when cart was emptied from another session'); + $user = Fixtures::get('existingUser'); + $product = Fixtures::get('product-1000'); + + $I->amGoingTo('add a product and go through till the last order step'); + $I->openShop() + ->loginUser( + $user['userLoginName'], + $user['userPassword'] + ); + $basket = new Basket($I); + $basket->addProductToBasket($product['OXID'], 1); + $orderCheckout = $basket->openMiniBasket()->openCheckout()->goToNextStep(); + + $I->amGoingTo('remove that product from basket in another session (browser tab)'); + $I->openNewTab(); + $I->openShop(); + (new Basket($I))->openMiniBasket() + ->openBasketDisplay() + ->updateProductAmount(0); + + $I->amGoingTo('go back to the initial session (tab)'); + $I->closeTab(); + + $I->amGoingTo('try to submit an order and make sure I see the warning'); + $orderCheckout->submitOrder(); + $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR')); + $I->see(Translator::translate('BASKET_EMPTY')); + + $I->amGoingTo('confirm that warning is gone after page reload'); + $I->reloadPage(); + $I->dontSee(Translator::translate('BASKET_ITEMS_CHANGED_ERROR')); + } + private function getExistingUserData(): array { return Fixtures::get('existingUser'); diff --git a/tests/Integration/Application/Controller/OrderControllerTest.php b/tests/Integration/Application/Controller/OrderControllerTest.php new file mode 100644 index 0000000000..4a9c380ac1 --- /dev/null +++ b/tests/Integration/Application/Controller/OrderControllerTest.php @@ -0,0 +1,136 @@ +prepareUserStub(); + $this->prepareBasketMock(); + $this->stubSession(); + unset($_SESSION['Errors']); + } + + public function testExecuteWithBasketMissingSummaryHashParameterWillLogAnError(): void + { + $logger = $this->createMock(LoggerInterface::class); + $this->injectLoggerMockIntoContainer($logger); + + $logger->expects($this->once()) + ->method('warning') + ->with($this->stringContains($this->basketSummaryHashParameter)); + + oxNew(OrderController::class)->execute(); + } + + public function testExecuteWithWrongBasketSummaryHashParameterAndEmptyBasketWillRedirectAndAddError(): void + { + $_GET[$this->basketSummaryHashParameter] = 'some-invalid-hash'; + $this->basket->method('getProductsCount')->willReturn(0); + + $redirect = oxNew(OrderController::class)->execute(); + + $this->assertEquals('basket', $redirect); + $this->assertNotEmpty($_SESSION['Errors']); + } + + public function testExecuteWithWrongBasketSummaryHashParameterAndNonEmptyBasketWillRedirectAndAddError(): void + { + $_GET[$this->basketSummaryHashParameter] = 'some-invalid-hash'; + $this->basket->method('getProductsCount')->willReturn(123); + + $redirect = oxNew(OrderController::class)->execute(); + + $this->assertEquals('order', $redirect); + $this->assertNotEmpty($_SESSION['Errors']); + } + + private function prepareUserStub(): void + { + Registry::getConfig()->setConfigParam('blEnableIntangibleProdAgreement', false); + $user = oxNew('oxUser'); + $user->oxuser__oxusername = new Field('some-user-name', Field::T_RAW); + $user->oxuser__oxpassword = new Field('some-user-pass', Field::T_RAW); + $user->save(); + + $this->userId = $user->getId(); + } + + private function stubSession(): void + { + $session = $this->createPartialMock( + Session::class, + [ + 'checkSessionChallenge', + 'getVariable', + 'getBasket', + ] + ); + $session->method('checkSessionChallenge') + ->willReturn(true); + $session->method('getBasket') + ->willReturn($this->basket); + $session->method('getVariable') + ->willReturnMap( + [ + ['login-token', null], + ['usr', $this->userId], + ] + ); + Registry::set(Session::class, $session); + } + + private function prepareBasketMock(): void + { + $this->basket = $this->createPartialMock( + Basket::class, + ['getProductsCount'] + ); + } + + private function injectLoggerMockIntoContainer(LoggerInterface $logger): void + { + $this->createContainer(); + $this->useNonVfsProjectConfigurationDirectory(); + $this->container->set(LoggerInterface::class, $logger); + $this->container->autowire(LoggerInterface::class, LoggerInterface::class); + $this->attachContainerToContainerFactory(); + } + + private function useNonVfsProjectConfigurationDirectory(): void + { + $this->container->get(ContextInterface::class) + ->setProjectConfigurationDirectory( + ContainerFacade::get(ContextInterface::class) + ->getProjectConfigurationDirectory() + ); + } +}