From 1fd1b1c0502438c24ac928eced3576940f5b53af Mon Sep 17 00:00:00 2001 From: Tadas Labutis Date: Thu, 27 Aug 2026 13:17:26 +0300 Subject: [PATCH 1/3] Resolve Fields flow from the field token, not the reported brand Work in progress, deliberately not ready for review. A shopper who selected one card brand and typed a card of another was left with an authorized payment and no order. On the return leg the module resolved the flow from the brand Saferpay reported instead of how the transaction was initialized, so a brand without saferpay_field.active never reached the Fields completion path and the checkout hung on Awaiting payment. Reloading then sent a second Authorize, which Saferpay refused with TRANSACTION_IN_WRONG_STATE, failing the order. PaymentTypeProvider::getForReturn() now decides from the field token and the saved-card selection, so any card typed into the Fields form completes. SaferPayTransactionProcessedGuard reads the authorized and captured flags off the saferpay_order row before the assert, and return.php takes the same lock key the notification uses, so a reload or a parallel notification cannot authorize twice. CardAliasRegistrationGuard centralises whether an alias is worth requesting, and AssertService::createObjectsFromAssertResponse() now takes that decision as a bool instead of re-deriving it from the posted card option. CardPaymentGroupingService::mergeCurrencies() limits the grouped Cards option to the currencies its own brands support. Deliberately left open, all four need closing before this becomes a PR: - SAFERPAY_GROUP_CARDS is flipped only in SaferPayConfig::getInstallDefaults(), which fresh installs read and nothing else does, so existing shops keep the per-brand list and the single Cards option does not actually ship. Needs an upgrade-script write of the configuration value. - Refusing a brand the merchant has not enabled is not implemented at all. InitializeRequest still strips PaymentMethods when a field token is present and inline-fields.js passes no brand restriction, so a disabled brand is accepted and authorized. - CardAliasRegistrationGuard::shouldRegister() returns !SAFERPAY_GROUP_CARDS, so defaulting grouping on silently disables saved cards. That contradicts keeping single-brand checkouts working as before and needs a product decision, not just code. - No test evidence yet on the reported scenario: no Visa-selected then Mastercard-paid run, no return-page reload test, no Payment Page regression pass. --- controllers/front/notify.php | 14 ++++ controllers/front/return.php | 38 +++++++++- src/Api/Request/AssertService.php | 8 +-- src/Config/SaferPayConfig.php | 2 +- src/Provider/PaymentTypeProvider.php | 20 ++++++ src/Service/CardAliasRegistrationGuard.php | 72 +++++++++++++++++++ src/Service/CardPaymentGroupingService.php | 30 +++++++- .../SaferPayTransactionAssertion.php | 2 +- .../SaferPayTransactionProcessedGuard.php | 68 ++++++++++++++++++ 9 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 src/Service/CardAliasRegistrationGuard.php create mode 100644 src/Service/TransactionFlow/SaferPayTransactionProcessedGuard.php diff --git a/controllers/front/notify.php b/controllers/front/notify.php index e1fc6bbf..d1ba9a73 100644 --- a/controllers/front/notify.php +++ b/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); diff --git a/controllers/front/return.php b/controllers/front/return.php index 366fd7ee..801a4762 100644 --- a/controllers/front/return.php +++ b/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,41 @@ 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. The same lock + // key is used by the notify controller, so whichever arrives second waits out the first. + $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 +140,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 { diff --git a/src/Api/Request/AssertService.php b/src/Api/Request/AssertService.php index c18949b8..e2ff6852 100644 --- a/src/Api/Request/AssertService.php +++ b/src/Api/Request/AssertService.php @@ -25,7 +25,6 @@ use Exception; use Invertus\SaferPay\Api\ApiRequest; -use Invertus\SaferPay\Config\SaferPayConfig; use Invertus\SaferPay\DTO\Request\Assert\AssertRequest; use Invertus\SaferPay\DTO\Response\Assert\AssertBody; use Invertus\SaferPay\EntityBuilder\SaferPayAssertBuilder; @@ -101,18 +100,19 @@ public function assert(AssertRequest $assertRequest, $isBusiness) * @param object|null $responseBody * @param int $saferPayOrderId * @param string $customerId - * @param int $selectedCardOption + * @param bool $saveCard whether an alias was requested from Saferpay for this transaction * * @return AssertBody * @throws Exception */ - public function createObjectsFromAssertResponse($responseBody, $saferPayOrderId, $customerId, $selectedCardOption) + public function createObjectsFromAssertResponse($responseBody, $saferPayOrderId, $customerId, $saveCard) { $assertBody = $this->assertResponseObjectCreator->createAssertObject($responseBody); $this->assertBuilder->createAssert($assertBody, $saferPayOrderId); $isPaymentSafe = $assertBody->getLiability()->getLiabilityShift(); - if ((int) $selectedCardOption === SaferPayConfig::CREDIT_CARD_OPTION_SAVE && $isPaymentSafe) { + // Storing a card alias is only possible when one was asked for, the response carries none otherwise. + if ($saveCard && $isPaymentSafe) { $this->aliasBuilder->createCardAlias($assertBody, $customerId); } diff --git a/src/Config/SaferPayConfig.php b/src/Config/SaferPayConfig.php index 9b36ad36..521f130d 100644 --- a/src/Config/SaferPayConfig.php +++ b/src/Config/SaferPayConfig.php @@ -445,7 +445,7 @@ public static function getDefaultConfiguration() self::SAFERPAY_PAYMENT_AWAITING ), self::SAFERPAY_SEND_ORDER_CONF_MAIL => 0, - self::SAFERPAY_GROUP_CARDS => 0, + self::SAFERPAY_GROUP_CARDS => 1, ]; } diff --git a/src/Provider/PaymentTypeProvider.php b/src/Provider/PaymentTypeProvider.php index 7cd7df74..c0278eea 100644 --- a/src/Provider/PaymentTypeProvider.php +++ b/src/Provider/PaymentTypeProvider.php @@ -58,6 +58,26 @@ public function get(string $paymentMethod): string return PaymentType::BASIC; } + /** + * Resolves the flow on the return leg from how the payment was initialized, not from the brand + * Saferpay reports back. A field token means the shopper paid through Saferpay Fields, whatever + * card they ended up typing into it. + * + * @param string $paymentMethod + * @param string|null $fieldToken + * @param bool $usingSavedCard + * + * @return string + */ + public function getForReturn(string $paymentMethod, $fieldToken = null, bool $usingSavedCard = false): string + { + if (!empty($fieldToken) || $usingSavedCard) { + return PaymentType::HOSTED_IFRAME; + } + + return $this->get($paymentMethod); + } + /** * @param string $paymentMethod * @return bool diff --git a/src/Service/CardAliasRegistrationGuard.php b/src/Service/CardAliasRegistrationGuard.php new file mode 100644 index 00000000..df3346f3 --- /dev/null +++ b/src/Service/CardAliasRegistrationGuard.php @@ -0,0 +1,72 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Service; + +use Invertus\SaferPay\Adapter\Configuration; +use Invertus\SaferPay\Config\SaferPayConfig; + +if (!defined('_PS_VERSION_')) { + exit; +} + +/** + * Asking Saferpay to register an alias only makes sense when the shop can offer that card back to + * the shopper later. The conditions below mirror the ones the checkout uses to display saved cards + * in SaferPayOfficial::hookPaymentOptions, so the shop never tokenises a card it cannot reuse. + */ +class CardAliasRegistrationGuard +{ + /** @var Configuration */ + private $configuration; + + public function __construct(Configuration $configuration) + { + $this->configuration = $configuration; + } + + /** + * @param mixed $selectedCard value posted by the checkout, 0 means the shopper opted in + * + * @return bool + */ + public function shouldRegister($selectedCard): bool + { + if ((int) $selectedCard !== SaferPayConfig::CREDIT_CARD_OPTION_SAVE) { + return false; + } + + if (!$this->configuration->getAsBoolean(SaferPayConfig::CREDIT_CARD_SAVE)) { + return false; + } + + // Saved cards are re-used through an alias transaction, which requires a business licence. + if (!$this->configuration->getAsBoolean(SaferPayConfig::BUSINESS_LICENSE . SaferPayConfig::getConfigSuffix())) { + return false; + } + + // With grouped cards the checkout shows a single "Cards" option, which carries no saved + // card list, so an alias registered under it could never be selected again. + return !$this->configuration->getAsBoolean(SaferPayConfig::SAFERPAY_GROUP_CARDS); + } +} diff --git a/src/Service/CardPaymentGroupingService.php b/src/Service/CardPaymentGroupingService.php index 66b242de..39c92340 100644 --- a/src/Service/CardPaymentGroupingService.php +++ b/src/Service/CardPaymentGroupingService.php @@ -63,10 +63,38 @@ public function group(array $paymentMethods, array $allCurrencies): array $result[] = [ 'paymentMethod' => SaferPayConfig::PAYMENT_CARDS, 'logoUrl' => _PS_BASE_URL_SSL_ . $this->module->getPathUri() . 'views/img/' . SaferPayConfig::PAYMENT_CARDS . '.png', - 'currencies' => $allCurrencies, + 'currencies' => $this->mergeCurrencies($cardMethods, $allCurrencies), ]; } return $result; } + + /** + * The grouped option may only offer the currencies its own card brands support. Handing it every + * shop currency would show the Cards option in a currency no enabled card can be paid in. + * + * @param array $cardMethods + * @param array $allCurrencies + * + * @return array + */ + private function mergeCurrencies(array $cardMethods, array $allCurrencies): array + { + $currencies = []; + + foreach ($cardMethods as $method) { + if (empty($method['currencies'])) { + continue; + } + + $currencies = array_merge($currencies, (array) $method['currencies']); + } + + if (empty($currencies)) { + return $allCurrencies; + } + + return array_values(array_unique($currencies)); + } } diff --git a/src/Service/TransactionFlow/SaferPayTransactionAssertion.php b/src/Service/TransactionFlow/SaferPayTransactionAssertion.php index 22732b10..f3b4294b 100644 --- a/src/Service/TransactionFlow/SaferPayTransactionAssertion.php +++ b/src/Service/TransactionFlow/SaferPayTransactionAssertion.php @@ -113,7 +113,7 @@ public function assert($cartId, $saveCard = null, $selectedCard = null, $isBusin $assertResponse, $saferPayOrder->id, $cart->id_customer, - $selectedCard + (bool) $saveCard ); // assertion shouldn't update, this is quickfix for what seems to be a general flaw in structure diff --git a/src/Service/TransactionFlow/SaferPayTransactionProcessedGuard.php b/src/Service/TransactionFlow/SaferPayTransactionProcessedGuard.php new file mode 100644 index 00000000..2955c2d2 --- /dev/null +++ b/src/Service/TransactionFlow/SaferPayTransactionProcessedGuard.php @@ -0,0 +1,68 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Service\TransactionFlow; + +use Invertus\SaferPay\Repository\SaferPayOrderRepository; +use SaferPayOrder; + +if (!defined('_PS_VERSION_')) { + exit; +} + +/** + * Saferpay requires a transaction to be marked as processed once a definite authorization response + * has been received, so the shop never authorizes the same transaction twice. The saferpay_order + * row carries that mark already, this only reads it before the assert is attempted again. + */ +class SaferPayTransactionProcessedGuard +{ + /** @var SaferPayOrderRepository */ + private $saferPayOrderRepository; + + public function __construct(SaferPayOrderRepository $saferPayOrderRepository) + { + $this->saferPayOrderRepository = $saferPayOrderRepository; + } + + /** + * A pending transaction is deliberately not treated as processed. Pending is not a definite + * response, the notification is still expected to settle it. + * + * @param int $cartId + * + * @return bool + */ + public function isProcessed(int $cartId): bool + { + $saferPayOrderId = (int) $this->saferPayOrderRepository->getIdByCartId($cartId); + + if (!$saferPayOrderId) { + return false; + } + + $saferPayOrder = new SaferPayOrder($saferPayOrderId); + + return (bool) $saferPayOrder->authorized || (bool) $saferPayOrder->captured; + } +} From 6ee66de9305b819db49ea89eac9a66160f67d37d Mon Sep 17 00:00:00 2001 From: Tadas Labutis Date: Fri, 28 Aug 2026 13:33:09 +0300 Subject: [PATCH 2/3] Group cards into one option and restrict Fields to enabled brands The Fields SDK accepted any brand the shopper typed because InitializeRequest strips PaymentMethods whenever a field token is set and inline-fields.js passed no restriction, so a Mastercard could be entered into a form opened as Visa. The companion fix already resolves the return flow from the field token; this narrows what the form accepts in the first place so a brand the merchant did not enable is refused at entry. EnabledCardBrandsProvider filters SaferPayConfig::CARD_BRANDS through PaymentRestrictionValidation, PaymentFormAssetLoader turns that into the SDK's lowercase brand names per payment option, and inline-fields.js feeds them to SaferpayFields.init as paymentMethods. InitializeRequest carries the same list to the Initialize call. AMEX joins FIELD_SUPPORTED_PAYMENT_METHODS. The grouped "Cards" option is now the default presentation, enabled for existing shops by the 2.1.0 upgrade. Supporting it meant teaching the rest of the checkout that PAYMENT_CARDS stands for several brands: BasePaymentRestrictionValidation resolves it to whether any enabled brand passes country and currency checks, SaferPayCardAliasRepository looks saved cards up across every enabled brand and the template prefixes each with the brand it was stored under, and CardAliasRegistrationGuard stops suppressing alias registration under grouping. return.php only takes the processing lock when the request is not the Saferpay notification, so the hosted payment page leg is no longer blocked by its own return page. V PAY and myOne have no SDK brand name, so a shop that enables either leaves its Fields form unrestricted rather than declining a card Saferpay would accept. Naming the real brand on an order paid through the grouped option is not included. --- changelog.md | 7 ++ controllers/front/ajax.php | 6 +- controllers/front/notify.php | 7 +- controllers/front/return.php | 28 ++++-- saferpayofficial.php | 16 ++- src/Config/SaferPayConfig.php | 16 +++ .../Request/Initialize/InitializeRequest.php | 29 +++++- .../Loader/PaymentFormAssetLoader.php | 65 +++++++++++- src/Provider/EnabledCardBrandsProvider.php | 65 ++++++++++++ .../SaferPayCardAliasRepository.php | 25 ++++- src/Service/CardAliasRegistrationGuard.php | 4 +- .../BasePaymentRestrictionValidation.php | 29 +++++- .../InitializeRequestObjectCreator.php | 16 ++- .../EnabledCardBrandsProviderTest.php | 72 ++++++++++++++ .../Unit/Provider/PaymentTypeProviderTest.php | 99 +++++++++++++++++++ tests/Unit/Provider/index.php | 31 ++++++ .../CardAliasRegistrationGuardTest.php | 89 +++++++++++++++++ .../BasePaymentRestrictionValidationTest.php | 98 +++++++++++++++++- upgrade/install-2.1.0.php | 12 +++ views/js/front/inline-fields.js | 70 ++++++++++--- .../hook/front/saferpay_additional_info.tpl | 2 +- 21 files changed, 740 insertions(+), 46 deletions(-) create mode 100644 src/Provider/EnabledCardBrandsProvider.php create mode 100644 tests/Unit/Provider/EnabledCardBrandsProviderTest.php create mode 100644 tests/Unit/Provider/PaymentTypeProviderTest.php create mode 100644 tests/Unit/Provider/index.php create mode 100644 tests/Unit/Service/CardAliasRegistrationGuardTest.php diff --git a/changelog.md b/changelog.md index ce9f6b82..adfb8d2b 100644 --- a/changelog.md +++ b/changelog.md @@ -218,3 +218,10 @@ - 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 diff --git a/controllers/front/ajax.php b/controllers/front/ajax.php index fb7b9ef6..8c1e2444 100644 --- a/controllers/front/ajax.php +++ b/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; } diff --git a/controllers/front/notify.php b/controllers/front/notify.php index d1ba9a73..12226634 100644 --- a/controllers/front/notify.php +++ b/controllers/front/notify.php @@ -156,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); diff --git a/controllers/front/return.php b/controllers/front/return.php index 801a4762..2053f98c 100644 --- a/controllers/front/return.php +++ b/controllers/front/return.php @@ -76,16 +76,21 @@ public function postProcess() } // 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. The same lock - // key is used by the notify controller, so whichever arrives second waits out the first. - $lockResult = $this->applyLock(sprintf('%s-%s', $cartId, $secureKey)); + // 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)); - // 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; + return; + } } /** @var SaferPayTransactionProcessedGuard $processedGuard */ @@ -360,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); diff --git a/saferpayofficial.php b/saferpayofficial.php index e5e44717..b7cad1eb 100644 --- a/saferpayofficial.php +++ b/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,9 +322,15 @@ 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 ); @@ -329,6 +338,7 @@ public function hookPaymentOptions($params) [ 'savedCards' => $savedCards, 'paymentMethod' => $paymentMethod['paymentMethod'], + 'showSavedCardBrand' => $paymentMethod['paymentMethod'] === SaferPayConfig::PAYMENT_CARDS, ] ); diff --git a/src/Config/SaferPayConfig.php b/src/Config/SaferPayConfig.php index 521f130d..0fe50a21 100644 --- a/src/Config/SaferPayConfig.php +++ b/src/Config/SaferPayConfig.php @@ -167,6 +167,7 @@ class SaferPayConfig ]; const FIELD_SUPPORTED_PAYMENT_METHODS = [ + self::PAYMENT_AMEX, self::PAYMENT_VISA, self::PAYMENT_VPAY, self::PAYMENT_MASTERCARD, @@ -314,6 +315,21 @@ class SaferPayConfig self::PAYMENT_BANCONTACT, ]; + /** + * Brand names the Saferpay Fields SDK accepts in its paymentMethods option, which are not the + * module's own constants. VPAY and MYONE have no SDK equivalent, so an option covering either + * cannot be restricted in the browser at all. + */ + public const FIELDS_SDK_BRANDS = [ + self::PAYMENT_AMEX => 'amex', + self::PAYMENT_BANCONTACT => 'bancontact', + self::PAYMENT_DINERS => 'diners', + self::PAYMENT_JCB => 'jcb', + self::PAYMENT_MAESTRO => 'maestro', + self::PAYMENT_MASTERCARD => 'mastercard', + self::PAYMENT_VISA => 'visa', + ]; + public static function supportsOrderCapture($paymentMethod) { //payments that DOES NOT SUPPORT capture diff --git a/src/DTO/Request/Initialize/InitializeRequest.php b/src/DTO/Request/Initialize/InitializeRequest.php index 17cae436..2da46ba7 100644 --- a/src/DTO/Request/Initialize/InitializeRequest.php +++ b/src/DTO/Request/Initialize/InitializeRequest.php @@ -117,6 +117,11 @@ class InitializeRequest implements SaferPayRequestInterface */ private $fieldToken; + /** + * @var array + */ + private $paymentMethods; + public function __construct( RequestHeader $requestHeader, $terminalId, @@ -132,7 +137,8 @@ public function __construct( $alias, Order $order, PayerProfile $payerProfile, - $fieldToken + $fieldToken, + array $paymentMethods = [] ) { $this->requestHeader = $requestHeader; $this->terminalId = $terminalId; @@ -149,6 +155,7 @@ public function __construct( $this->order = $order; $this->payerProfile = $payerProfile; $this->fieldToken = $fieldToken; + $this->paymentMethods = $paymentMethods; } public function getAsArray() @@ -166,9 +173,7 @@ public function getAsArray() 'ClientInfo' => $this->requestHeader->getClientInfo(), ], 'TerminalId' => $this->terminalId, - 'PaymentMethods' => [ - $this->paymentMethod, - ], + 'PaymentMethods' => $this->getPaymentMethods(), 'Payment' => [ 'Amount' => [ 'Value' => $this->payment->getValue(), @@ -268,6 +273,22 @@ public function getAsArray() return $return; } + /** + * The grouped "Cards" option is sent to Saferpay as the brands the merchant actually enabled. + * Sending its own "CARD" value instead would let Saferpay offer every brand the terminal + * supports, including ones switched off in the back office. + * + * @return array + */ + private function getPaymentMethods() + { + if (!empty($this->paymentMethods)) { + return $this->paymentMethods; + } + + return [$this->paymentMethod]; + } + /** * @return array */ diff --git a/src/Presentation/Loader/PaymentFormAssetLoader.php b/src/Presentation/Loader/PaymentFormAssetLoader.php index f4a517ec..9284fc02 100644 --- a/src/Presentation/Loader/PaymentFormAssetLoader.php +++ b/src/Presentation/Loader/PaymentFormAssetLoader.php @@ -28,6 +28,7 @@ use Invertus\SaferPay\Enum\ControllerName; use Invertus\SaferPay\Enum\PaymentType; use Invertus\SaferPay\Factory\ModuleFactory; +use Invertus\SaferPay\Provider\EnabledCardBrandsProvider; use Invertus\SaferPay\Provider\OpcModulesProvider; use Invertus\SaferPay\Service\SaferPayErrorDisplayService; use Media; @@ -46,12 +47,19 @@ class PaymentFormAssetLoader private $context; /** @var OpcModulesProvider $opcModuleProvider */ private $opcModulesProvider; + /** @var EnabledCardBrandsProvider */ + private $enabledCardBrandsProvider; - public function __construct(ModuleFactory $module, LegacyContext $context, OpcModulesProvider $opcModulesProvider) - { + public function __construct( + ModuleFactory $module, + LegacyContext $context, + OpcModulesProvider $opcModulesProvider, + EnabledCardBrandsProvider $enabledCardBrandsProvider + ) { $this->module = $module->getModule(); $this->context = $context; $this->opcModulesProvider = $opcModulesProvider; + $this->enabledCardBrandsProvider = $enabledCardBrandsProvider; } public function register($controller) @@ -186,6 +194,7 @@ private function registerInlineFieldsAssets($controller) 'saferpay_field_label_cardnumber' => $this->module->l('Card number', 'PaymentFormAssetLoader'), 'saferpay_field_label_expiration' => $this->module->l('Expiry date', 'PaymentFormAssetLoader'), 'saferpay_field_label_cvc' => $this->module->l('CVC', 'PaymentFormAssetLoader'), + 'saferpay_field_payment_methods' => $this->getFieldPaymentMethods(), ]); $controller->registerJavascript( @@ -201,6 +210,58 @@ private function registerInlineFieldsAssets($controller) ); } + /** + * Which card brands the Fields form may accept, keyed by the checkout option that renders it. + * A brand typed into an option that does not list it is rejected by the SDK before submit, + * which is what stops a Mastercard being paid under a Visa-only option. + * + * @return array + */ + private function getFieldPaymentMethods() + { + $enabledBrands = $this->enabledCardBrandsProvider->get(); + $methods = []; + + foreach ($enabledBrands as $brand) { + if (!isset(SaferPayConfig::FIELDS_SDK_BRANDS[$brand])) { + continue; + } + + $methods[$brand] = [SaferPayConfig::FIELDS_SDK_BRANDS[$brand]]; + } + + $groupedBrands = $this->getGroupedFieldBrands($enabledBrands); + + if ($groupedBrands) { + $methods[SaferPayConfig::PAYMENT_CARDS] = $groupedBrands; + } + + return $methods; + } + + /** + * Empty as soon as one enabled brand has no SDK equivalent: a partial allowlist would decline + * a card Saferpay itself accepts, so the grouped option is left unrestricted instead. + * + * @param array $enabledBrands + * + * @return array + */ + private function getGroupedFieldBrands(array $enabledBrands) + { + $brands = []; + + foreach ($enabledBrands as $brand) { + if (!isset(SaferPayConfig::FIELDS_SDK_BRANDS[$brand])) { + return []; + } + + $brands[] = SaferPayConfig::FIELDS_SDK_BRANDS[$brand]; + } + + return $brands; + } + public function registerErrorBags() { /** @var SaferPayErrorDisplayService $errorDisplayService */ diff --git a/src/Provider/EnabledCardBrandsProvider.php b/src/Provider/EnabledCardBrandsProvider.php new file mode 100644 index 00000000..2d05571d --- /dev/null +++ b/src/Provider/EnabledCardBrandsProvider.php @@ -0,0 +1,65 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Provider; + +use Invertus\SaferPay\Config\SaferPayConfig; +use Invertus\SaferPay\Service\PaymentRestrictionValidation; + +if (!defined('_PS_VERSION_')) { + exit; +} + +class EnabledCardBrandsProvider +{ + /** + * @var PaymentRestrictionValidation + */ + private $paymentRestrictionValidation; + + public function __construct(PaymentRestrictionValidation $paymentRestrictionValidation) + { + $this->paymentRestrictionValidation = $paymentRestrictionValidation; + } + + /** + * Card brands the shop can actually be paid with right now: enabled in the back office and + * passing the country and currency restrictions for the current context. + * + * @return array + */ + public function get(): array + { + $brands = []; + + foreach (SaferPayConfig::CARD_BRANDS as $brand) { + if (!$this->paymentRestrictionValidation->isPaymentMethodValid($brand)) { + continue; + } + + $brands[] = $brand; + } + + return $brands; + } +} diff --git a/src/Repository/SaferPayCardAliasRepository.php b/src/Repository/SaferPayCardAliasRepository.php index 70fb0da7..1c1a3117 100644 --- a/src/Repository/SaferPayCardAliasRepository.php +++ b/src/Repository/SaferPayCardAliasRepository.php @@ -32,13 +32,32 @@ class SaferPayCardAliasRepository { - public function getSavedValidCardsByUserIdAndPaymentMethod($userId, $paymentMethod, $currentDate) + /** + * The grouped "Cards" option covers several brands at once, and an alias is always stored + * under the brand Saferpay reported, never under "CARD". The brand comes back with the row so + * the checkout can tell two saved cards of different brands apart. + * + * @param int $userId + * @param array $paymentMethods + * @param string $currentDate + * + * @return array + */ + public function getSavedValidCardsByUserIdAndPaymentMethods($userId, array $paymentMethods, $currentDate) { + if (empty($paymentMethods)) { + return []; + } + + $escapedMethods = array_map(function ($paymentMethod) { + return '"' . pSQL($paymentMethod) . '"'; + }, $paymentMethods); + $query = new DbQuery(); - $query->select('`id_saferpay_card_alias`, `card_number`'); + $query->select('`id_saferpay_card_alias`, `card_number`, `payment_method`'); $query->from('saferpay_card_alias'); $query->where('id_customer = ' . (int) $userId); - $query->where('payment_method = "' . pSQL($paymentMethod) . '"'); + $query->where('payment_method IN (' . implode(', ', $escapedMethods) . ')'); $query->where('valid_till > "' . pSQL($currentDate) . '"'); return Db::getInstance()->executeS($query); diff --git a/src/Service/CardAliasRegistrationGuard.php b/src/Service/CardAliasRegistrationGuard.php index df3346f3..95e5de46 100644 --- a/src/Service/CardAliasRegistrationGuard.php +++ b/src/Service/CardAliasRegistrationGuard.php @@ -65,8 +65,6 @@ public function shouldRegister($selectedCard): bool return false; } - // With grouped cards the checkout shows a single "Cards" option, which carries no saved - // card list, so an alias registered under it could never be selected again. - return !$this->configuration->getAsBoolean(SaferPayConfig::SAFERPAY_GROUP_CARDS); + return true; } } diff --git a/src/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidation.php b/src/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidation.php index f664ae2b..599b4761 100644 --- a/src/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidation.php +++ b/src/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidation.php @@ -76,7 +76,7 @@ public function __construct( public function isValid(string $paymentName): bool { if ($paymentName === SaferPayConfig::PAYMENT_CARDS) { - return true; + return $this->hasAnyEnabledCardBrand(); } if (!$this->isPaymentMethodEnabled($paymentName)) { @@ -104,6 +104,33 @@ public function supports(string $paymentName): bool return true; } + /** + * The grouped "Cards" option carries no restrictions of its own. It is payable exactly when at + * least one of the brands behind it is enabled and passes the country and currency checks. + * + * @return bool + */ + private function hasAnyEnabledCardBrand() + { + foreach (SaferPayConfig::CARD_BRANDS as $brand) { + if (!$this->isPaymentMethodEnabled($brand)) { + continue; + } + + if (!$this->isCountrySupportedByPaymentName($brand)) { + continue; + } + + if (!$this->isCurrencySupportedByPaymentName($brand)) { + continue; + } + + return true; + } + + return false; + } + /** * @param string $paymentName * diff --git a/src/Service/Request/InitializeRequestObjectCreator.php b/src/Service/Request/InitializeRequestObjectCreator.php index 4278d9ed..e3beaef4 100644 --- a/src/Service/Request/InitializeRequestObjectCreator.php +++ b/src/Service/Request/InitializeRequestObjectCreator.php @@ -29,6 +29,7 @@ use Invertus\SaferPay\Config\SaferPayConfig; use Invertus\SaferPay\DTO\Request\Initialize\InitializeRequest; use Invertus\SaferPay\DTO\Request\Payer; +use Invertus\SaferPay\Provider\EnabledCardBrandsProvider; if (!defined('_PS_VERSION_')) { exit; @@ -41,9 +42,17 @@ class InitializeRequestObjectCreator */ private $requestObjectCreator; - public function __construct(RequestObjectCreator $requestObjectCreator) - { + /** + * @var EnabledCardBrandsProvider + */ + private $enabledCardBrandsProvider; + + public function __construct( + RequestObjectCreator $requestObjectCreator, + EnabledCardBrandsProvider $enabledCardBrandsProvider + ) { $this->requestObjectCreator = $requestObjectCreator; + $this->enabledCardBrandsProvider = $enabledCardBrandsProvider; } public function create( @@ -104,7 +113,8 @@ public function create( $alias, $order, $payerProfile, - $fieldToken + $fieldToken, + $paymentMethod === SaferPayConfig::PAYMENT_CARDS ? $this->enabledCardBrandsProvider->get() : [] ); } } diff --git a/tests/Unit/Provider/EnabledCardBrandsProviderTest.php b/tests/Unit/Provider/EnabledCardBrandsProviderTest.php new file mode 100644 index 00000000..b0da72a4 --- /dev/null +++ b/tests/Unit/Provider/EnabledCardBrandsProviderTest.php @@ -0,0 +1,72 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Tests\Unit\Provider; + +use Invertus\SaferPay\Config\SaferPayConfig; +use Invertus\SaferPay\Provider\EnabledCardBrandsProvider; +use Invertus\SaferPay\Service\PaymentRestrictionValidation; +use Invertus\SaferPay\Tests\Unit\Tools\UnitTestCase; + +class EnabledCardBrandsProviderTest extends UnitTestCase +{ + public function testItReturnsEveryBrandWhenNoneIsRestricted() + { + $provider = new EnabledCardBrandsProvider($this->mockValidation(SaferPayConfig::CARD_BRANDS)); + + $this->assertEquals(SaferPayConfig::CARD_BRANDS, $provider->get()); + } + + public function testItSkipsRestrictedBrandsAndKeepsTheConfiguredOrder() + { + $validBrands = [SaferPayConfig::PAYMENT_MASTERCARD, SaferPayConfig::PAYMENT_VISA]; + + $provider = new EnabledCardBrandsProvider($this->mockValidation($validBrands)); + + $this->assertEquals($validBrands, $provider->get()); + } + + public function testItReturnsNothingWhenEveryBrandIsRestricted() + { + $provider = new EnabledCardBrandsProvider($this->mockValidation([])); + + $this->assertEquals([], $provider->get()); + } + + private function mockValidation(array $validBrands) + { + $validationMock = $this + ->getMockBuilder(PaymentRestrictionValidation::class) + ->disableOriginalConstructor() + ->getMock(); + + $validationMock + ->method('isPaymentMethodValid') + ->willReturnCallback(function ($paymentMethod) use ($validBrands) { + return in_array($paymentMethod, $validBrands, true); + }) + ; + + return $validationMock; + } +} diff --git a/tests/Unit/Provider/PaymentTypeProviderTest.php b/tests/Unit/Provider/PaymentTypeProviderTest.php new file mode 100644 index 00000000..09746458 --- /dev/null +++ b/tests/Unit/Provider/PaymentTypeProviderTest.php @@ -0,0 +1,99 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Tests\Unit\Provider; + +use Invertus\SaferPay\Config\SaferPayConfig; +use Invertus\SaferPay\Enum\PaymentType; +use Invertus\SaferPay\Provider\PaymentTypeProvider; +use Invertus\SaferPay\Repository\SaferPayFieldRepository; +use Invertus\SaferPay\Tests\Unit\Tools\UnitTestCase; + +class PaymentTypeProviderTest extends UnitTestCase +{ + public function testItResolvesFieldsFromTheFieldTokenWhateverBrandCameBack() + { + $provider = $this->mockProviderExpectingNoBrandLookup(); + + $this->assertEquals( + PaymentType::HOSTED_IFRAME, + $provider->getForReturn(SaferPayConfig::PAYMENT_MASTERCARD, 'field-token', false) + ); + } + + public function testItResolvesFieldsForASavedCard() + { + $provider = $this->mockProviderExpectingNoBrandLookup(); + + $this->assertEquals( + PaymentType::HOSTED_IFRAME, + $provider->getForReturn(SaferPayConfig::PAYMENT_CARDS, null, true) + ); + } + + public function testItFallsBackToTheBrandLookupWhenNeitherIsPresent() + { + $provider = $this->mockProvider(); + + $provider + ->expects($this->once()) + ->method('get') + ->with(SaferPayConfig::PAYMENT_VISA) + ->willReturn(PaymentType::BASIC) + ; + + $this->assertEquals( + PaymentType::BASIC, + $provider->getForReturn(SaferPayConfig::PAYMENT_VISA, null, false) + ); + } + + private function mockProviderExpectingNoBrandLookup() + { + $provider = $this->mockProvider(); + + $provider + ->expects($this->never()) + ->method('get') + ; + + return $provider; + } + + /** + * get() reads the module configuration directly, so it is stubbed out to keep getForReturn + * testable without a configured shop. + */ + private function mockProvider() + { + $fieldRepositoryMock = $this + ->getMockBuilder(SaferPayFieldRepository::class) + ->getMock(); + + return $this + ->getMockBuilder(PaymentTypeProvider::class) + ->setConstructorArgs([$fieldRepositoryMock]) + ->setMethods(['get']) + ->getMock(); + } +} diff --git a/tests/Unit/Provider/index.php b/tests/Unit/Provider/index.php new file mode 100644 index 00000000..ee622726 --- /dev/null +++ b/tests/Unit/Provider/index.php @@ -0,0 +1,31 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ +header('Expires: Mon, 26 Jul 1997 05:00:00 GMT'); +header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT'); + +header('Cache-Control: no-store, no-cache, must-revalidate'); +header('Cache-Control: post-check=0, pre-check=0', false); +header('Pragma: no-cache'); + +header('Location: ../'); +exit; diff --git a/tests/Unit/Service/CardAliasRegistrationGuardTest.php b/tests/Unit/Service/CardAliasRegistrationGuardTest.php new file mode 100644 index 00000000..41f16eaa --- /dev/null +++ b/tests/Unit/Service/CardAliasRegistrationGuardTest.php @@ -0,0 +1,89 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Tests\Unit\Service; + +use Invertus\SaferPay\Adapter\Configuration; +use Invertus\SaferPay\Config\SaferPayConfig; +use Invertus\SaferPay\Service\CardAliasRegistrationGuard; +use Invertus\SaferPay\Tests\Unit\Tools\UnitTestCase; + +class CardAliasRegistrationGuardTest extends UnitTestCase +{ + public function testItRegistersWhenTheShopperOptedIn() + { + $guard = new CardAliasRegistrationGuard($this->mockConfiguration(true, true)); + + $this->assertTrue($guard->shouldRegister(SaferPayConfig::CREDIT_CARD_OPTION_SAVE)); + } + + public function testItDoesNotRegisterWhenTheShopperPaysWithANewCardOnce() + { + $guard = new CardAliasRegistrationGuard($this->mockConfiguration(true, true)); + + $this->assertFalse($guard->shouldRegister(SaferPayConfig::CREDIT_CARD_DONT_OPTION_SAVE)); + } + + public function testItDoesNotRegisterWhenTheShopperPaysWithASavedCard() + { + $guard = new CardAliasRegistrationGuard($this->mockConfiguration(true, true)); + + $this->assertFalse($guard->shouldRegister(7)); + } + + public function testItDoesNotRegisterWhenCardSavingIsDisabled() + { + $guard = new CardAliasRegistrationGuard($this->mockConfiguration(false, true)); + + $this->assertFalse($guard->shouldRegister(SaferPayConfig::CREDIT_CARD_OPTION_SAVE)); + } + + public function testItDoesNotRegisterWithoutABusinessLicence() + { + $guard = new CardAliasRegistrationGuard($this->mockConfiguration(true, false)); + + $this->assertFalse($guard->shouldRegister(SaferPayConfig::CREDIT_CARD_OPTION_SAVE)); + } + + private function mockConfiguration($cardSavingEnabled, $hasBusinessLicence) + { + $configurationMock = $this + ->getMockBuilder(Configuration::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationMock + ->method('getAsBoolean') + ->willReturnMap([ + [SaferPayConfig::CREDIT_CARD_SAVE, null, $cardSavingEnabled], + [ + SaferPayConfig::BUSINESS_LICENSE . SaferPayConfig::getConfigSuffix(), + null, + $hasBusinessLicence, + ], + ]) + ; + + return $configurationMock; + } +} diff --git a/tests/Unit/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidationTest.php b/tests/Unit/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidationTest.php index 0b87f9fc..63d70ccd 100644 --- a/tests/Unit/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidationTest.php +++ b/tests/Unit/Service/PaymentRestrictionValidation/BasePaymentRestrictionValidationTest.php @@ -24,7 +24,11 @@ namespace Invertus\SaferPay\Tests\Unit\Service\PaymentRestrictionValidation; use Invertus\SaferPay\Config\SaferPayConfig; +use Invertus\SaferPay\Repository\SaferPayPaymentRepository; +use Invertus\SaferPay\Repository\SaferPayRestrictionRepository; use Invertus\SaferPay\Service\PaymentRestrictionValidation\BasePaymentRestrictionValidation; +use Invertus\SaferPay\Service\SaferPayObtainPaymentMethods; +use Invertus\SaferPay\Service\SaferPayRestrictionCreator; use Invertus\SaferPay\Tests\Unit\Tools\UnitTestCase; class BasePaymentRestrictionValidationTest extends UnitTestCase @@ -41,7 +45,8 @@ public function testIsValid( $basePaymentRestrictionValidation = new BasePaymentRestrictionValidation( $this->mockContext('AT', 'AUD'), $this->getPaymentRepositoryMock($paymentName, $paymentResults), - $this->getRestrictionRepositoryMock($paymentName, $restrictionResults) + $this->getRestrictionRepositoryMock($paymentName, $restrictionResults), + $this->getObtainPaymentMethodsMock() ); $this->assertEquals($expectedResult, $basePaymentRestrictionValidation->isValid($paymentName)); } @@ -114,4 +119,95 @@ public function getBasePaymentRestrictionValidationDataProvider() ], ]; } + + /** + * @dataProvider getGroupedCardsDataProvider + */ + public function testItValidatesGroupedCardsThroughTheBrandsBehindThem( + $enabledBrands, + $enabledCountries, + $expectedResult + ) { + $basePaymentRestrictionValidation = new BasePaymentRestrictionValidation( + $this->mockContext('AT', 'AUD'), + $this->getBrandPaymentRepositoryMock($enabledBrands), + $this->getBrandRestrictionRepositoryMock($enabledCountries), + $this->getObtainPaymentMethodsMock() + ); + + $this->assertEquals( + $expectedResult, + $basePaymentRestrictionValidation->isValid(SaferPayConfig::PAYMENT_CARDS) + ); + } + + public function getGroupedCardsDataProvider() + { + return [ + [ + 'enabledBrands' => [SaferPayConfig::PAYMENT_VISA], + 'enabledCountries' => [0], //ALL COUNTRIES + 'expectedResult' => true, + ], + [ + 'enabledBrands' => SaferPayConfig::CARD_BRANDS, + 'enabledCountries' => [0], //ALL COUNTRIES + 'expectedResult' => true, + ], + [ + 'enabledBrands' => [], //EVERY BRAND DISABLED + 'enabledCountries' => [0], //ALL COUNTRIES + 'expectedResult' => false, + ], + [ + 'enabledBrands' => SaferPayConfig::CARD_BRANDS, + 'enabledCountries' => [], //NO COUNTRIES + 'expectedResult' => false, + ], + ]; + } + + private function getBrandPaymentRepositoryMock(array $enabledBrands) + { + $paymentRepositoryMock = $this + ->getMockBuilder(SaferPayPaymentRepository::class) + ->getMock(); + + $paymentRepositoryMock + ->method('isActiveByName') + ->willReturnCallback(function ($paymentName) use ($enabledBrands) { + return in_array($paymentName, $enabledBrands, true); + }) + ; + + return $paymentRepositoryMock; + } + + private function getBrandRestrictionRepositoryMock(array $enabledCountries) + { + $restrictionMock = $this + ->getMockBuilder(SaferPayRestrictionRepository::class) + ->getMock(); + + $restrictionMock + ->method('getSelectedIdsByName') + ->willReturnCallback(function ($paymentName, $restrictionType) use ($enabledCountries) { + if ($restrictionType === SaferPayRestrictionCreator::RESTRICTION_COUNTRY) { + return $enabledCountries; + } + + return [0]; //ALL CURRENCIES + }) + ; + + return $restrictionMock; + } + + private function getObtainPaymentMethodsMock() + { + return $this + ->getMockBuilder(SaferPayObtainPaymentMethods::class) + ->disableOriginalConstructor() + ->getMock(); + } } diff --git a/upgrade/install-2.1.0.php b/upgrade/install-2.1.0.php index 1fbb4f6d..0c9ca95d 100644 --- a/upgrade/install-2.1.0.php +++ b/upgrade/install-2.1.0.php @@ -30,6 +30,7 @@ function upgrade_module_2_1_0() saferpayofficial_2_1_0_delete_removed_tabs(); saferpayofficial_2_1_0_delete_removed_files(); saferpayofficial_2_1_0_delete_removed_configuration(); + saferpayofficial_2_1_0_enable_card_grouping(); Tools::clearSmartyCache(); @@ -154,3 +155,14 @@ function saferpayofficial_2_1_0_delete_removed_configuration() { Configuration::deleteByName('SAFERPAY_HOSTED_FIELDS_TEMPLATE'); } + +/** + * Card brands are no longer offered one by one in the checkout. A shopper who picked a brand and + * then typed a card of another one had the payment approved by Saferpay but no order created, so + * the single "Cards" option becomes the way card payments render. Shops upgrading from an earlier + * version have the setting at 0 and would otherwise keep the per-brand list and the defect with it. + */ +function saferpayofficial_2_1_0_enable_card_grouping() +{ + Configuration::updateValue('SAFERPAY_GROUP_CARDS', 1); +} diff --git a/views/js/front/inline-fields.js b/views/js/front/inline-fields.js index aa4bdad8..0c2ab995 100644 --- a/views/js/front/inline-fields.js +++ b/views/js/front/inline-fields.js @@ -155,6 +155,20 @@ return parseInt($form.find('[name="selectedCreditCard_' + method + '"]').val(), 10) || 0; } + // Brands this option's Fields form may accept, as the SDK's own lowercase names. Absent + // when the merchant enabled a brand the SDK cannot express (VPAY, myOne): a partial list + // would decline a card Saferpay itself accepts, so the form is left unrestricted. + function fieldPaymentMethods($form) { + if (typeof saferpay_field_payment_methods === 'undefined') { + return null; + } + + var method = $form.find('[name="saved_card_method"]').val(); + var brands = saferpay_field_payment_methods[method]; + + return (brands && brands.length) ? brands : null; + } + function stopLoading() { if (loadingTimeout) { clearTimeout(loadingTimeout); @@ -172,7 +186,7 @@ // Render a fresh Fields form into the selected option's container and initialise the SDK // on it. Rebuilding fresh readonly-input placeholders each time keeps re-initialisation // valid when the customer switches between card options. - function renderInto($container) { + function renderInto($container, $form) { var containerId = $container.attr('id'); if (renderedContainerId === containerId && $('#' + SLOT_ID).length) { return; @@ -185,7 +199,7 @@ renderedContainerId = containerId; loadingTimeout = setTimeout(stopLoading, LOADING_TIMEOUT); - SaferpayFields.init({ + var fieldsConfig = { accessToken: saferpay_field_access_token, url: saferpay_field_url, // Visible labels sit in the field border notch (see fieldMarkup), so the inputs @@ -266,7 +280,40 @@ toggleFieldClass(evt.fieldType, 'is-focused', false); } } - }); + }; + + var allowedBrands = fieldPaymentMethods($form); + if (allowedBrands) { + fieldsConfig.paymentMethods = allowedBrands; + } + + SaferpayFields.init(fieldsConfig); + } + + // The saved-card radios are rendered into the option's additional-information block, which + // is a sibling of the pay-with-