SL-386 Fix orders lost when paying with a different card brand than selected

changelog.md

@@ -210,11 +210,20 @@
 ## [2.1.0]
 - BO : Redesigned back-office settings into a React single-page admin
 - BO : Conditionally show/hide Saferpay Fields settings based on account license
-- BO : Renamed the "Custom form" column in Payment methods to "Saferpay Fields"
+- BO : Replaced the per-brand "Saferpay Fields" toggles with a single "Use Saferpay Fields" setting, initialized from the old toggles during upgrade
+- FO : Card payments automatically fall back to the Saferpay Payment Page when the Saferpay Fields access token is missing
+- Added automatic Saferpay Fields access token generation from stored API credentials during upgrade
 - BO/FO : Accessibility improvements for EAA / WCAG 2.1 AA compliance
 - Added configurable payment description and order reference on payment page
 - API update to V1.50: added WERO and GIFTCARD payment methods, removed deprecated GIROPAY/PAYDIREKT/SOFORT
 - BO : Fixed issue when a freshly installed module logged an account error before any API credentials were entered
 - BO : Fixed issue when the "Could not reach your Saferpay account" warning kept showing after payment methods had loaded successfully
 - Fixed issue when files removed in this version stayed on disk after an upgrade, leaving obsolete iframe checkout controllers reachable and re-creating obsolete menu tabs on module reset
 - BO : Fixed issue when a saved API password offered no visible way to enter a new one, and browser password manager icons covered the show/hide password control
+- FO : Replaced the per-brand card list in checkout with a single "Cards" option that accepts only the enabled brands, with saved cards and Saferpay Fields support
+- FO : Added American Express support to the Saferpay Fields form
+- FO : Fixed issue when paying with a card brand other than the one selected left the payment authorized at Saferpay without a completed order
+- FO : Fixed issue when reloading the payment return page sent a second authorization and failed an already paid order
+- FO : Fixed issue when the payment behavior without 3-D Secure setting was ignored for orders paid through the grouped "Cards" option
+- FO : Fixed issue when the grouped "Cards" option ignored payment method country and currency restrictions
+- FO : Fixed issue when an order paid through the Saferpay hosted payment page stayed awaiting payment because the return page blocked the Saferpay notification

controllers/admin/AdminSaferPayOfficialSettingsController.php

@@ -22,13 +22,11 @@
  */
 
 use Invertus\SaferPay\Config\SaferPayConfig;
-use Invertus\SaferPay\Repository\SaferPayFieldRepository;
 use Invertus\SaferPay\Repository\SaferPayLogoRepository;
 use Invertus\SaferPay\Repository\SaferPayPaymentRepository;
 use Invertus\SaferPay\Repository\SaferPayRestrictionRepository;
 use Invertus\SaferPay\Repository\SaferPaySavedCreditCardRepository;
 use Invertus\SaferPay\Adapter\Configuration as SaferPayConfiguration;
-use Invertus\SaferPay\Service\SaferPayFieldCreator;
 use Invertus\SaferPay\Service\SaferPayGenerateFieldAccessToken;
 use Invertus\SaferPay\Service\SaferPayGetLicense;
 use Invertus\SaferPay\Service\SaferPayGetTerminals;
@@ -97,9 +95,11 @@ public function setMedia($isNewTheme = false)
     {
         parent::setMedia($isNewTheme);
 
+        // The bundle filename never changes between releases, so CDNs keep serving the
+        // previous version's build after an upgrade unless the URL carries the version.
         $distPath = 'modules/' . $this->module->name . '/views/js/admin/dist/';
-        $this->addJS($distPath . 'saferpay-settings.js');
-        $this->addCSS($distPath . 'saferpay-settings.css');
+        $this->addJS($distPath . 'saferpay-settings.js?v=' . $this->module->version);
+        $this->addCSS($distPath . 'saferpay-settings.css?v=' . $this->module->version);
     }
 
     public function initContent()
@@ -318,6 +318,7 @@ public function ajaxProcessSavePaymentProcessing()
         $configuration->set(SaferPayConfig::SAFERPAY_ORDER_CREATION_AFTER_AUTHORIZATION, $this->getIntValue($data, 'orderCreationAfterAuth'));
         $configuration->set(SaferPayConfig::SAFERPAY_GROUP_CARDS, !empty($data['groupCards']) ? 1 : 0);
         $configuration->set(SaferPayConfig::SAFERPAY_GROUP_CARDS_LOGO, !empty($data['groupCardsLogo']) ? 1 : 0);
+        $configuration->set(SaferPayConfig::SAFERPAY_USE_FIELDS, !empty($data['useFields']) ? 1 : 0);
         $configuration->set(SaferPayConfig::CREDIT_CARD_SAVE, $this->getIntValue($data, 'creditCardSave'));
 
         // If credit card save disabled, clean up saved cards
@@ -407,9 +408,6 @@ public function ajaxProcessSavePaymentMethods()
         /** @var SaferPayLogoCreator $logoCreation */
         $logoCreation = $this->module->getService(SaferPayLogoCreator::class);
 
-        /** @var SaferPayFieldCreator $fieldCreation */
-        $fieldCreation = $this->module->getService(SaferPayFieldCreator::class);
-
         /** @var SaferPayRestrictionCreator $restrictionCreator */
         $restrictionCreator = $this->module->getService(SaferPayRestrictionCreator::class);
 
@@ -422,7 +420,6 @@ public function ajaxProcessSavePaymentMethods()
             $paymentName = $method['name'];
             $success = $paymentCreation->updatePayment($paymentName, !empty($method['enabled'])) && $success;
             $success = $logoCreation->updateLogo($paymentName, !empty($method['showLogos'])) && $success;
-            $success = $fieldCreation->updateField($paymentName, !empty($method['showCustomForm'])) && $success;
 
             try {
                 $countries = isset($method['countries']) ? $method['countries'] : [];
@@ -574,6 +571,11 @@ private function collectSettingsData()
         // Resolved before the payload is built because it sets $paymentMethodsFetchFailed.
         $paymentMethodsData = $this->getPaymentMethodsData();
 
+        $envSuffix = $configuration->get(SaferPayConfig::TEST_MODE) ? SaferPayConfig::TEST_SUFFIX : '';
+        $fieldsAccessTokenMissing = (bool) $configuration->get(SaferPayConfig::BUSINESS_LICENSE . $envSuffix)
+            && (bool) $configuration->get(SaferPayConfig::SAFERPAY_USE_FIELDS)
+            && !$configuration->get(SaferPayConfig::FIELDS_ACCESS_TOKEN . $envSuffix);
+
         $data = [
             // Environment
             'testMode' => (bool) $configuration->get(SaferPayConfig::TEST_MODE),
@@ -605,6 +607,8 @@ private function collectSettingsData()
             'orderCreationAfterAuth' => (int) $configuration->get(SaferPayConfig::SAFERPAY_ORDER_CREATION_AFTER_AUTHORIZATION),
             'groupCards' => (bool) $configuration->get(SaferPayConfig::SAFERPAY_GROUP_CARDS),
             'groupCardsLogo' => (bool) $configuration->get(SaferPayConfig::SAFERPAY_GROUP_CARDS_LOGO),
+            'useFields' => (bool) $configuration->get(SaferPayConfig::SAFERPAY_USE_FIELDS),
+            'fieldsAccessTokenMissing' => $fieldsAccessTokenMissing,
             'creditCardSave' => (int) $configuration->get(SaferPayConfig::CREDIT_CARD_SAVE),
 
             // Email
@@ -751,9 +755,6 @@ private function getPaymentMethodsData()
         /** @var SaferPayLogoRepository $logoRepository */
         $logoRepository = $this->module->getService(SaferPayLogoRepository::class);
 
-        /** @var SaferPayFieldRepository $fieldRepository */
-        $fieldRepository = $this->module->getService(SaferPayFieldRepository::class);
-
         /** @var SaferPayRestrictionRepository $restrictionRepository */
         $restrictionRepository = $this->module->getService(SaferPayRestrictionRepository::class);
 
@@ -767,8 +768,6 @@ private function getPaymentMethodsData()
                 'displayName' => $saferPayPaymentNotation->getForDisplay($paymentMethod),
                 'enabled' => (bool) $paymentRepository->isActiveByName($paymentMethod),
                 'showLogos' => (bool) $logoRepository->isActiveByName($paymentMethod),
-                'showCustomForm' => (bool) $fieldRepository->isActiveByName($paymentMethod),
-                'hasCustomForm' => in_array($paymentMethod, SaferPayConfig::FIELD_SUPPORTED_PAYMENT_METHODS),
                 'countries' => $restrictionRepository->getSelectedIdsByName(
                     $paymentMethod,
                     SaferPayRestrictionCreator::RESTRICTION_COUNTRY

controllers/front/ajax.php

@@ -106,7 +106,7 @@ protected function processGetStatus()
             'isFinished' => $saferPayOrder->authorized || $saferPayOrder->captured || $saferPayOrder->pending,
             'href' => $this->context->link->getModuleLink(
                 $this->module->name,
-                $this->getSuccessControllerName($isBusinessLicence, $fieldToken),
+                $this->getSuccessControllerName($isBusinessLicence, $fieldToken, (int) $selectedCard > 0),
                 [
                     'cartId' => $cartId,
                     'orderId' => $saferPayOrder->id_order,
@@ -132,9 +132,9 @@ private function getFailControllerLink($cartId, $secureKey, $moduleId)
         );
     }
 
-    private function getSuccessControllerName($isBusinessLicence, $fieldToken)
+    private function getSuccessControllerName($isBusinessLicence, $fieldToken, $usingSavedCard)
     {
-        if ($fieldToken) {
+        if ($fieldToken || $usingSavedCard) {
             return ControllerName::SUCCESS_HOSTED;
         }
 

controllers/front/notify.php

@@ -30,6 +30,7 @@
 use Invertus\SaferPay\Repository\SaferPayOrderRepository;
 use Invertus\SaferPay\Service\SaferPayOrderStatusService;
 use Invertus\SaferPay\Service\TransactionFlow\SaferPayTransactionAssertion;
+use Invertus\SaferPay\Service\TransactionFlow\SaferPayTransactionProcessedGuard;
 use Invertus\SaferPay\Utility\ExceptionUtility;
 
 if (!defined('_PS_VERSION_')) {
@@ -110,6 +111,19 @@ public function postProcess()
             die($this->module->l('Order already complete', self::FILE_NAME));
         }
 
+        /** @var SaferPayTransactionProcessedGuard $processedGuard */
+        $processedGuard = $this->module->getService(SaferPayTransactionProcessedGuard::class);
+
+        if ($processedGuard->isProcessed($cartId)) {
+            $logger->debug(sprintf('%s - Payment already processed. Dying.', self::FILE_NAME), [
+                'context' => [
+                    'cart_id' => $cartId,
+                ],
+            ]);
+
+            die($this->module->l('Order already complete', self::FILE_NAME));
+        }
+
         /** @var SaferPayOrderRepository $saferPayOrderRepository */
         $saferPayOrderRepository = $this->module->getService(SaferPayOrderRepository::class);
 
@@ -142,8 +156,13 @@ public function postProcess()
 
             $paymentBehaviorWithout3D = (int) Configuration::get(SaferPayConfig::PAYMENT_BEHAVIOR_WITHOUT_3D);
 
+            // $order->payment holds the checkout option's name, which is "Cards" for the grouped
+            // option and never matches a brand, silently skipping the whole without-3DS behaviour.
+            // The brand Saferpay asserted is what this setting is about.
+            $assertedPaymentMethod = $assertResponseBody->getPaymentMeans()->getBrand()->getPaymentMethod();
+
             if (!$assertResponseBody->getLiability()->getLiabilityShift() &&
-                in_array($order->payment, SaferPayConfig::SUPPORTED_3DS_PAYMENT_METHODS)
+                in_array($assertedPaymentMethod, SaferPayConfig::SUPPORTED_3DS_PAYMENT_METHODS)
             ) {
                 /** @var SaferPayOrderStatusService $orderStatusService */
                 $orderStatusService = $this->module->getService(SaferPayOrderStatusService::class);

controllers/front/return.php

@@ -33,9 +33,12 @@
 use Invertus\SaferPay\Processor\CheckoutProcessor;
 use Invertus\SaferPay\Provider\PaymentTypeProvider;
 use Invertus\SaferPay\Repository\SaferPayFieldRepository;
+use Invertus\SaferPay\Response\Response;
+use Invertus\SaferPay\Service\CardAliasRegistrationGuard;
 use Invertus\SaferPay\Service\SaferPayOrderStatusService;
 use Invertus\SaferPay\Service\TransactionFlow\SaferPayTransactionAssertion;
 use Invertus\SaferPay\Service\TransactionFlow\SaferPayTransactionAuthorization;
+use Invertus\SaferPay\Service\TransactionFlow\SaferPayTransactionProcessedGuard;
 use Invertus\SaferPay\Utility\ExceptionUtility;
 use Invertus\SaferPay\Adapter\Cart as CartAdapter;
 
@@ -72,16 +75,46 @@ public function postProcess()
             $this->redirectWithNotifications($this->getRedirectionToControllerUrl($failController));
         }
 
+        // Saferpay sends the redirect and the notification in parallel, and with a business licence
+        // the assert below authorizes the transaction, which may only ever happen once, so both legs
+        // share a lock key. With isWebhook set the notification is the only leg that completes the
+        // payment and the assert here is the read-only PaymentPage/Assert, so taking the lock would
+        // only starve the notification, which dies on a conflict instead of waiting and leaves the
+        // order awaiting payment forever.
+        if (!Tools::getValue('isWebhook')) {
+            $lockResult = $this->applyLock(sprintf('%s-%s', $cartId, $secureKey));
+
+            // Only a conflict means the notification holds the lock. Any other failure is the locking
+            // itself being unavailable, and the processed check below still guards the repeated assert.
+            if ($lockResult->getStatusCode() === Response::HTTP_CONFLICT) {
+                $logger->debug(sprintf('%s - Notification is already being processed, skipping assert', self::FILE_NAME));
+
+                return;
+            }
+        }
+
+        /** @var SaferPayTransactionProcessedGuard $processedGuard */
+        $processedGuard = $this->module->getService(SaferPayTransactionProcessedGuard::class);
+
+        if ($processedGuard->isProcessed($cartId)) {
+            $logger->debug(sprintf('%s - Payment already processed, skipping assert', self::FILE_NAME));
+
+            return;
+        }
+
         /** @var SaferPayTransactionAssertion $transactionAssert */
         $transactionAssert = $this->module->getService(SaferPayTransactionAssertion::class);
 
+        /** @var CardAliasRegistrationGuard $aliasRegistrationGuard */
+        $aliasRegistrationGuard = $this->module->getService(CardAliasRegistrationGuard::class);
+
         $assertResponseBody = null;
         $transactionStatus = null;
 
         try {
             $assertResponseBody = $transactionAssert->assert(
                 $cartId,
-                (int) $selectedCard === SaferPayConfig::CREDIT_CARD_OPTION_SAVE,
+                $aliasRegistrationGuard->shouldRegister($selectedCard),
                 $selectedCard,
                 (int) Tools::getValue(SaferPayConfig::IS_BUSINESS_LICENCE)
             );
@@ -112,7 +145,13 @@ public function postProcess()
         /** @var PaymentTypeProvider $paymentTypeProvider */
         $paymentTypeProvider = $this->module->getService(PaymentTypeProvider::class);
 
-        if ($paymentTypeProvider->get($orderPayment) === PaymentType::HOSTED_IFRAME) {
+        $paymentType = $paymentTypeProvider->getForReturn(
+            $orderPayment,
+            Tools::getValue('fieldToken'),
+            (int) $selectedCard > 0
+        );
+
+        if ($paymentType === PaymentType::HOSTED_IFRAME) {
             $order = new Order(Order::getIdByCartId($cartId));
 
             try {
@@ -326,8 +365,11 @@ private function createAndValidateOrder($assertResponseBody, $transactionStatus,
         $order = new Order($orderId);
         $paymentBehaviorWithout3D = (int) Configuration::get(SaferPayConfig::PAYMENT_BEHAVIOR_WITHOUT_3D);
 
+        // $order->payment holds the checkout option's name, which is "Cards" for the grouped
+        // option and never matches a brand, silently skipping the whole without-3DS behaviour.
+        // The brand Saferpay asserted is what this setting is about.
         if (!$assertResponseBody->getLiability()->getLiabilityShift() &&
-            in_array($order->payment, SaferPayConfig::SUPPORTED_3DS_PAYMENT_METHODS)
+            in_array($orderPayment, SaferPayConfig::SUPPORTED_3DS_PAYMENT_METHODS)
         ) {
             /** @var SaferPayOrderStatusService $orderStatusService */
             $orderStatusService = $this->module->getService(SaferPayOrderStatusService::class);

saferpayofficial.php

@@ -25,6 +25,7 @@
 use Invertus\SaferPay\Presentation\Loader\PaymentFormAssetLoader;
 use Invertus\SaferPay\Presenter\AdminOrderPagePresenter;
 use Invertus\SaferPay\Presenter\AssertPresenter;
+use Invertus\SaferPay\Provider\EnabledCardBrandsProvider;
 use Invertus\SaferPay\Provider\PaymentRedirectionProvider;
 use Invertus\SaferPay\Repository\SaferPayCardAliasRepository;
 use Invertus\SaferPay\Repository\SaferPayOrderRepository;
@@ -261,6 +262,8 @@ public function hookPaymentOptions($params)
         $paymentRedirectionProvider = $this->getService(PaymentRedirectionProvider::class);
         /** @var LegacyTranslator $translator */
         $translator = $this->getService(LegacyTranslator::class);
+        /** @var EnabledCardBrandsProvider $enabledCardBrandsProvider */
+        $enabledCardBrandsProvider = $this->getService(EnabledCardBrandsProvider::class);
 
         $isBusinessLicenseEnabled = Configuration::get(SaferPayConfig::BUSINESS_LICENSE . SaferPayConfig::getConfigSuffix());
         $isCreditCardSavingEnabled = Configuration::get(SaferPayConfig::CREDIT_CARD_SAVE);
@@ -289,7 +292,7 @@ public function hookPaymentOptions($params)
             $isCreditCard = in_array(
                 $paymentMethod['paymentMethod'],
                 SaferPayConfig::TRANSACTION_METHODS
-            );
+            ) || $paymentMethod['paymentMethod'] === SaferPayConfig::PAYMENT_CARDS;
 
             $selectedCard = 0;
             $isCreditCardSavingEnabledForUser = $isCreditCardSavingEnabled;
@@ -319,16 +322,23 @@ public function hookPaymentOptions($params)
             if ($isCreditCardSavingEnabledForUser && $isCreditCard && $isBusinessLicenseEnabled) {
                 $currentDate = date('Y-m-d h:i:s');
 
-                $savedCards = $cardAliasRepository->getSavedValidCardsByUserIdAndPaymentMethod(
+                // Aliases are stored under the brand Saferpay reported, so the grouped "Cards"
+                // option has to look up every brand it stands for, not its own "CARD" name.
+                $savedCardBrands = $paymentMethod['paymentMethod'] === SaferPayConfig::PAYMENT_CARDS
+                    ? $enabledCardBrandsProvider->get()
+                    : [$paymentMethod['paymentMethod']];
+
+                $savedCards = $cardAliasRepository->getSavedValidCardsByUserIdAndPaymentMethods(
                     $this->context->customer->id,
-                    $paymentMethod['paymentMethod'],
+                    $savedCardBrands,
                     $currentDate
                 );
 
                 $this->smarty->assign(
                     [
                         'savedCards' => $savedCards,
                         'paymentMethod' => $paymentMethod['paymentMethod'],
+                        'showSavedCardBrand' => $paymentMethod['paymentMethod'] === SaferPayConfig::PAYMENT_CARDS,
                     ]
                 );