From e4c442d7e947619ab12e25bcacb94c2644120f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 24 Aug 2026 11:41:18 +0200 Subject: [PATCH 1/3] Surface Quickpay's fraud signals Three surfaces for the fraud data the SDK already models: - The admin operation history panel shows a "Fraud suspected" badge (translated in all 16 locales) next to the test-mode badge, read off the payment's metadata. - An opt-in fraud.block_capture config flag makes PaymentProcessor consult the new Fraud/FraudChecker before the automatic capture on the complete transition and skip it with a warning when Quickpay reports the payment as fraud suspected - the transition itself proceeds, leaving the payment for manual review. The checker fails open on every unanswerable case (no Quickpay payment, no api key, Quickpay unreachable), so it never blocks the payment flow, and it is off by default because it costs one extra API call per automatic capture. - setono:sylius-quickpay:reconcile-payments gains a --fraud-suspected report mode that queries each configured gateway's account directly for flagged payments in the period - report only, nothing is transitioned. The final SDK endpoint classes cannot be doubled, so the new tests run the real SDK client against a canned-response PSR-18 stub (tests/Quickpay/ FixedResponseHttpClient), and the report mode was verified live against the Quickpay API. Closes #132 --- CLAUDE.md | 7 +- README.md | 20 +++ UPGRADE-2.0.md | 4 + src/Command/ReconcilePaymentsCommand.php | 72 +++++++++ src/DependencyInjection/Configuration.php | 10 ++ .../SetonoSyliusQuickpayExtension.php | 3 +- src/Fraud/FraudChecker.php | 62 ++++++++ src/Fraud/FraudCheckerInterface.php | 17 +++ src/Resources/config/services.xml | 16 ++ src/Resources/translations/messages.cs.yaml | 1 + src/Resources/translations/messages.da.yaml | 1 + src/Resources/translations/messages.de.yaml | 1 + src/Resources/translations/messages.en.yaml | 1 + src/Resources/translations/messages.es.yaml | 1 + src/Resources/translations/messages.fi.yaml | 1 + src/Resources/translations/messages.fr.yaml | 1 + src/Resources/translations/messages.hu.yaml | 1 + src/Resources/translations/messages.it.yaml | 1 + src/Resources/translations/messages.nl.yaml | 1 + src/Resources/translations/messages.no.yaml | 1 + src/Resources/translations/messages.pl.yaml | 1 + src/Resources/translations/messages.pt.yaml | 1 + src/Resources/translations/messages.ro.yaml | 1 + src/Resources/translations/messages.sv.yaml | 1 + src/Resources/translations/messages.uk.yaml | 1 + .../order/show/payment/_operations.html.twig | 3 + src/StateMachine/PaymentProcessor.php | 15 ++ .../Command/ReconcilePaymentsCommandTest.php | 57 +++++++ .../DependencyInjection/ConfigurationTest.php | 32 ++++ .../SetonoSyliusQuickpayExtensionTest.php | 1 + tests/Fraud/FraudCheckerTest.php | 141 ++++++++++++++++++ tests/Quickpay/FixedResponseHttpClient.php | 25 ++++ tests/StateMachine/PaymentProcessorTest.php | 73 +++++++-- 33 files changed, 561 insertions(+), 13 deletions(-) create mode 100644 src/Fraud/FraudChecker.php create mode 100644 src/Fraud/FraudCheckerInterface.php create mode 100644 tests/Fraud/FraudCheckerTest.php create mode 100644 tests/Quickpay/FixedResponseHttpClient.php diff --git a/CLAUDE.md b/CLAUDE.md index 75232c7..f9279d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,7 +140,12 @@ but does not block the transition. Operations execute with the Sylius payment th gateway-updated details persist; an unqualified `Refund` refunds Quickpay's remaining `balance` and the balance is persisted into the details by the library's Status/Confirm/Sync actions. Each operation can be turned off via the plugin config `operations.capture` / `operations.refund` / `operations.cancel` (defined in `DependencyInjection/Configuration.php`, -passed to the processor as container parameters). +passed to the processor as container parameters). An opt-in `fraud.block_capture` flag makes the processor consult +`Fraud/FraudChecker` (fetches the payment via `Quickpay/ClientFactory`, reads `metadata.fraud_suspected`, fails +open on any error) before the automatic capture and skip it with a warning when fraud is suspected — the +transition itself proceeds. The operation-history admin panel shows a fraud badge from the same metadata, and the +reconcile command has a `--fraud-suspected` report mode that queries each configured gateway's account directly +(`PaymentsQuery`), transitioning nothing. ### Payment link (admin) `PaymentLink/PaymentLinkProvider` returns Sylius' `sylius_shop_order_pay` url (built for the order's channel diff --git a/README.md b/README.md index a01df99..4531bd6 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,8 @@ setono_sylius_quickpay: capture: true # forward the payment's complete transition to Quickpay as a capture refund: true # forward the refund transition to Quickpay cancel: true # forward the cancel transition to Quickpay + fraud: + block_capture: false # skip the automatic capture when Quickpay reports the payment as fraud suspected (see Fraud signals) ``` The plugin hooks into **both state machine adapters** supported by Sylius 1.14 — a `winzou_state_machine` @@ -204,6 +206,24 @@ captured balance and a test-mode badge. The data is fetched from Quickpay *after so the order page is never delayed by a slow gateway; if Quickpay cannot be reached, the panel shows an inline notice with a retry link. Nothing is stored — the panel reflects what Quickpay reports right now. +## Fraud signals + +Quickpay flags payments it suspects of fraud, and the plugin surfaces that in three places: + +- A **Fraud suspected** badge on the admin operation history panel, next to the test-mode badge. +- An **opt-in capture guard**: with `fraud.block_capture: true` (see [Configure the plugin](#3-configure-the-plugin-optional)), + the automatic capture on the payment's `complete` transition is skipped when Quickpay reports the payment as + fraud suspected — the completion itself is not blocked; the payment is logged and left for manual review, so + you capture or cancel it in the Quickpay manager after looking at it. The check asks Quickpay at capture time + and fails open: an unreachable Quickpay never blocks the payment flow. It costs one extra API call per + automatic capture, which is why it is off by default. +- A **report mode** on the reconciliation command that asks Quickpay for every payment flagged in the period, + regardless of its local state — report only, nothing is transitioned: + +```bash +bin/console setono:sylius-quickpay:reconcile-payments --fraud-suspected --since="7 days" +``` + ## Callbacks All callbacks for what your store does arrive on a **per-payment url** the gateway mints and registers itself — diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index f0791f7..8c177f0 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -159,6 +159,10 @@ if you make those and want the store to notice. The README's *Callbacks* section - **New: a reconciliation command.** `setono:sylius-quickpay:reconcile-payments` polls Quickpay for payments stuck in a non-final state — e.g. because a callback never arrived — and applies the matching payment transition. See the README for options and a suggested cron cadence. +- **New: Quickpay's fraud signals are surfaced.** The admin operation history shows a *Fraud suspected* + badge, the reconciliation command gains a `--fraud-suspected` report mode, and an opt-in + `fraud.block_capture` config flag skips the automatic capture on completion for fraud suspected + payments, leaving them for manual review. - **New: the API key is verified at form-save time.** Saving a Quickpay payment method pings the Quickpay API with the submitted key and rejects the form when Quickpay rejects it; an unreachable Quickpay skips the check, so an outage never blocks saving. diff --git a/src/Command/ReconcilePaymentsCommand.php b/src/Command/ReconcilePaymentsCommand.php index 4ffd535..ebb02e1 100644 --- a/src/Command/ReconcilePaymentsCommand.php +++ b/src/Command/ReconcilePaymentsCommand.php @@ -9,13 +9,17 @@ use Payum\Core\Payum; use Payum\Core\Request\GetHumanStatus; use Setono\Doctrine\ORMTrait; +use Setono\Payum\Quickpay\QuickpayGatewayFactory; use Setono\Quickpay\Exception\QuickpayException; +use Setono\Quickpay\Request\Payment\PaymentsQuery; use Setono\SyliusQuickpayPlugin\Provider\PendingPaymentProviderInterface; +use Setono\SyliusQuickpayPlugin\Quickpay\ClientFactoryInterface; use Sylius\Abstraction\StateMachine\StateMachineInterface; use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Payment\PaymentTransitions; +use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -36,10 +40,15 @@ final class ReconcilePaymentsCommand extends Command { use ORMTrait; + /** + * @param RepositoryInterface $gatewayConfigRepository + */ public function __construct( private readonly PendingPaymentProviderInterface $pendingPaymentProvider, private readonly Payum $payum, private readonly StateMachineInterface $stateMachine, + private readonly ClientFactoryInterface $clientFactory, + private readonly RepositoryInterface $gatewayConfigRepository, ManagerRegistry $managerRegistry, ) { parent::__construct(); @@ -53,6 +62,7 @@ protected function configure(): void ->addOption('since', null, InputOption::VALUE_REQUIRED, 'Only reconcile payments created within this period', '7 days') ->addOption('limit', null, InputOption::VALUE_REQUIRED, 'Maximum number of payments to check', '100') ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Report what would happen without applying any transition') + ->addOption('fraud-suspected', null, InputOption::VALUE_NONE, 'Report the payments Quickpay flags as fraud suspected instead of reconciling; no transition is applied') ; } @@ -74,6 +84,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $limit = (int) $input->getOption('limit'); $dryRun = (bool) $input->getOption('dry-run'); + if (true === $input->getOption('fraud-suspected')) { + return $this->reportFraudSuspected($io, $createdSince, $limit); + } + $checked = $transitioned = $unchanged = $errored = 0; foreach ($this->pendingPaymentProvider->findPending($createdSince, $limit) as $payment) { @@ -119,6 +133,64 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $errored > 0 ? Command::FAILURE : Command::SUCCESS; } + /** + * Unlike reconciliation, which walks the local pending payments, this asks Quickpay directly: + * every configured Quickpay gateway is queried for payments flagged fraud suspected within the + * period, regardless of their local state. Report only — nothing is transitioned or persisted. + */ + private function reportFraudSuspected(SymfonyStyle $io, \DateTimeImmutable $createdSince, int $limit): int + { + $rows = []; + $errored = false; + + /** @var GatewayConfigInterface $gatewayConfig */ + foreach ($this->gatewayConfigRepository->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) as $gatewayConfig) { + $config = $gatewayConfig->getConfig(); + + // Configurations written by the 1.x form may still carry the old key + $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; + if (!is_string($apiKey) || '' === $apiKey) { + continue; + } + + try { + $payments = $this->clientFactory->create($apiKey)->payments()->paginate(new PaymentsQuery( + minTime: $createdSince, + fraudSuspected: true, + sortBy: 'created_at', + sortDir: 'desc', + )); + + foreach ($payments as $payment) { + $rows[] = [ + (string) $gatewayConfig->getGatewayName(), + (string) $payment->id, + $payment->orderId, + $payment->state, + $payment->createdAt?->format('Y-m-d H:i:s') ?? '', + $payment->testMode ? 'yes' : 'no', + ]; + + if (\count($rows) >= $limit) { + break 2; + } + } + } catch (QuickpayException $e) { + $errored = true; + $io->warning(sprintf('Gateway "%s": %s', (string) $gatewayConfig->getGatewayName(), $e->getMessage())); + } + } + + if ([] === $rows) { + $io->success('Quickpay reports no fraud suspected payments in the period.'); + } else { + $io->table(['Gateway', 'Quickpay id', 'Order id', 'State', 'Created', 'Test mode'], $rows); + $io->warning(sprintf('%d payment(s) flagged as fraud suspected. Review them in the Quickpay manager before capturing.', \count($rows))); + } + + return $errored ? Command::FAILURE : Command::SUCCESS; + } + private static function resolveGatewayName(PaymentInterface $payment): string { /** @var PaymentMethodInterface $method */ diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index e3747fe..4ef93e1 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -36,6 +36,16 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->end() + ->arrayNode('fraud') + ->info("Reacting to Quickpay's fraud signals") + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('block_capture') + ->info('Skip the automatic capture on the complete transition when Quickpay reports the payment as fraud suspected, leaving it for manual review in the Quickpay manager (the transition itself is not blocked). Costs one extra Quickpay API call per automatic capture') + ->defaultFalse() + ->end() + ->end() + ->end() ->arrayNode('checkout') ->info('Presentation of Quickpay payment methods on the checkout payment step') ->addDefaultsIfNotSet() diff --git a/src/DependencyInjection/SetonoSyliusQuickpayExtension.php b/src/DependencyInjection/SetonoSyliusQuickpayExtension.php index 66e1b5a..a7f4a57 100644 --- a/src/DependencyInjection/SetonoSyliusQuickpayExtension.php +++ b/src/DependencyInjection/SetonoSyliusQuickpayExtension.php @@ -15,13 +15,14 @@ final class SetonoSyliusQuickpayExtension extends Extension implements PrependEx { public function load(array $configs, ContainerBuilder $container): void { - /** @var array{operations: array{capture: bool, refund: bool, cancel: bool}, checkout: array{payment_method_logos: array, creditcard_brands: list}} $config */ + /** @var array{operations: array{capture: bool, refund: bool, cancel: bool}, fraud: array{block_capture: bool}, checkout: array{payment_method_logos: array, creditcard_brands: list}} $config */ $config = $this->processConfiguration($this->getConfiguration([], $container), $configs); $loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $container->setParameter('setono_sylius_quickpay.operations.capture', $config['operations']['capture']); $container->setParameter('setono_sylius_quickpay.operations.refund', $config['operations']['refund']); $container->setParameter('setono_sylius_quickpay.operations.cancel', $config['operations']['cancel']); + $container->setParameter('setono_sylius_quickpay.fraud.block_capture', $config['fraud']['block_capture']); $container->setParameter('setono_sylius_quickpay.checkout.payment_method_logos', $config['checkout']['payment_method_logos']); $container->setParameter('setono_sylius_quickpay.checkout.creditcard_brands', $config['checkout']['creditcard_brands']); diff --git a/src/Fraud/FraudChecker.php b/src/Fraud/FraudChecker.php new file mode 100644 index 0000000..1217b29 --- /dev/null +++ b/src/Fraud/FraudChecker.php @@ -0,0 +1,62 @@ +getDetails()['quickpayPaymentId'] ?? null; + if (!is_numeric($quickpayPaymentId)) { + return false; + } + + $apiKey = self::resolveApiKey($payment); + if (null === $apiKey) { + return false; + } + + try { + $quickpayPayment = $this->clientFactory->create($apiKey)->payments()->getById((int) $quickpayPaymentId); + } catch (\Throwable $e) { + // An unreachable Quickpay must not block the payment flow: report the payment as clean + $this->logger?->warning(sprintf('Could not check the Quickpay payment for suspected fraud: %s', $e->getMessage()), [ + 'quickpayPaymentId' => (int) $quickpayPaymentId, + 'paymentId' => $payment->getId(), + ]); + + return false; + } + + return true === $quickpayPayment->metadata?->fraudSuspected; + } + + private static function resolveApiKey(PaymentInterface $payment): ?string + { + $method = $payment->getMethod(); + if (!$method instanceof PaymentMethodInterface) { + return null; + } + + $config = $method->getGatewayConfig()?->getConfig() ?? []; + + // Configurations written by the 1.x form may still carry the old key + $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; + + return is_string($apiKey) && '' !== $apiKey ? $apiKey : null; + } +} diff --git a/src/Fraud/FraudCheckerInterface.php b/src/Fraud/FraudCheckerInterface.php new file mode 100644 index 0000000..f8e9f3c --- /dev/null +++ b/src/Fraud/FraudCheckerInterface.php @@ -0,0 +1,17 @@ + + + @@ -119,12 +121,26 @@ alias="setono_sylius_quickpay.notify_idempotency"/> + + + + + + + + + + + + %setono_sylius_quickpay.operations.capture% %setono_sylius_quickpay.operations.refund% %setono_sylius_quickpay.operations.cancel% + %setono_sylius_quickpay.fraud.block_capture% diff --git a/src/Resources/translations/messages.cs.yaml b/src/Resources/translations/messages.cs.yaml index 43349de..1246928 100644 --- a/src/Resources/translations/messages.cs.yaml +++ b/src/Resources/translations/messages.cs.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Zatím žádné operace' balance: 'Stržený zůstatek' test_mode: 'Testovací režim' + fraud_suspected: 'Podezření na podvod' payment_link: 'Platební odkaz' payment_link_help: 'Kdokoli s tímto odkazem může objednávku zaplatit v platebním okně Quickpay — pošlete ho zákazníkovi pro zaplacení telefonické objednávky nebo objednávky na fakturu, případně po opuštěné pokladně.' copy: 'Kopírovat' diff --git a/src/Resources/translations/messages.da.yaml b/src/Resources/translations/messages.da.yaml index 487f06d..da003ce 100644 --- a/src/Resources/translations/messages.da.yaml +++ b/src/Resources/translations/messages.da.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Ingen operationer endnu' balance: 'Hævet saldo' test_mode: 'Testtilstand' + fraud_suspected: 'Mistanke om svindel' payment_link: 'Betalingslink' payment_link_help: 'Alle med dette link kan betale ordren i Quickpays betalingsvindue — send det til kunden for at opkræve betaling for en telefon- eller fakturaordre, eller efter en afbrudt checkout.' copy: 'Kopiér' diff --git a/src/Resources/translations/messages.de.yaml b/src/Resources/translations/messages.de.yaml index 4871ef1..59f5293 100644 --- a/src/Resources/translations/messages.de.yaml +++ b/src/Resources/translations/messages.de.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Noch keine Vorgänge' balance: 'Erfasster Saldo' test_mode: 'Testmodus' + fraud_suspected: 'Betrugsverdacht' payment_link: 'Zahlungslink' payment_link_help: 'Jeder mit diesem Link kann die Bestellung im Quickpay-Zahlungsfenster bezahlen — senden Sie ihn dem Kunden, um eine Telefon- oder Rechnungsbestellung einzuziehen oder nach einem abgebrochenen Checkout.' copy: 'Kopieren' diff --git a/src/Resources/translations/messages.en.yaml b/src/Resources/translations/messages.en.yaml index 3264ca2..10f9a3f 100644 --- a/src/Resources/translations/messages.en.yaml +++ b/src/Resources/translations/messages.en.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'No operations yet' balance: 'Captured balance' test_mode: 'Test mode' + fraud_suspected: 'Fraud suspected' payment_link: 'Payment link' payment_link_help: 'Anyone with this link can pay this order in the Quickpay payment window — send it to the customer to collect payment for a phone or invoice order, or after an abandoned checkout.' copy: 'Copy' diff --git a/src/Resources/translations/messages.es.yaml b/src/Resources/translations/messages.es.yaml index 0bbfb3c..0ca48b9 100644 --- a/src/Resources/translations/messages.es.yaml +++ b/src/Resources/translations/messages.es.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Aún no hay operaciones' balance: 'Saldo capturado' test_mode: 'Modo de prueba' + fraud_suspected: 'Sospecha de fraude' payment_link: 'Enlace de pago' payment_link_help: 'Cualquiera con este enlace puede pagar el pedido en la ventana de pago de Quickpay — envíeselo al cliente para cobrar un pedido telefónico o con factura, o tras un checkout abandonado.' copy: 'Copiar' diff --git a/src/Resources/translations/messages.fi.yaml b/src/Resources/translations/messages.fi.yaml index e8918bc..3b089ba 100644 --- a/src/Resources/translations/messages.fi.yaml +++ b/src/Resources/translations/messages.fi.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Ei tapahtumia vielä' balance: 'Veloitettu saldo' test_mode: 'Testitila' + fraud_suspected: 'Petosepäily' payment_link: 'Maksulinkki' payment_link_help: 'Kuka tahansa, jolla on tämä linkki, voi maksaa tilauksen Quickpayn maksuikkunassa — lähetä se asiakkaalle puhelin- tai laskutilauksen maksamiseksi tai keskeytyneen kassan jälkeen.' copy: 'Kopioi' diff --git a/src/Resources/translations/messages.fr.yaml b/src/Resources/translations/messages.fr.yaml index 4aa260d..444c0f2 100644 --- a/src/Resources/translations/messages.fr.yaml +++ b/src/Resources/translations/messages.fr.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Aucune opération pour le moment' balance: 'Solde capturé' test_mode: 'Mode test' + fraud_suspected: 'Suspicion de fraude' payment_link: 'Lien de paiement' payment_link_help: 'Toute personne disposant de ce lien peut payer la commande dans la fenêtre de paiement Quickpay — envoyez-le au client pour encaisser une commande par téléphone ou sur facture, ou après un paiement abandonné.' copy: 'Copier' diff --git a/src/Resources/translations/messages.hu.yaml b/src/Resources/translations/messages.hu.yaml index a24bb95..6c0a2e2 100644 --- a/src/Resources/translations/messages.hu.yaml +++ b/src/Resources/translations/messages.hu.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Még nincsenek műveletek' balance: 'Terhelt egyenleg' test_mode: 'Tesztmód' + fraud_suspected: 'Csalásgyanú' payment_link: 'Fizetési link' payment_link_help: 'Bárki, aki rendelkezik ezzel a linkkel, kifizetheti a rendelést a Quickpay fizetési ablakában — küldje el a vevőnek telefonos vagy számlás rendelés beszedéséhez, vagy megszakított fizetés után.' copy: 'Másolás' diff --git a/src/Resources/translations/messages.it.yaml b/src/Resources/translations/messages.it.yaml index 5876ff3..8b84544 100644 --- a/src/Resources/translations/messages.it.yaml +++ b/src/Resources/translations/messages.it.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Nessuna operazione al momento' balance: 'Saldo catturato' test_mode: 'Modalità test' + fraud_suspected: 'Sospetta frode' payment_link: 'Link di pagamento' payment_link_help: 'Chiunque abbia questo link può pagare l''ordine nella finestra di pagamento Quickpay — invialo al cliente per incassare un ordine telefonico o con fattura, oppure dopo un checkout abbandonato.' copy: 'Copia' diff --git a/src/Resources/translations/messages.nl.yaml b/src/Resources/translations/messages.nl.yaml index 2fec1cb..cdf2376 100644 --- a/src/Resources/translations/messages.nl.yaml +++ b/src/Resources/translations/messages.nl.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Nog geen bewerkingen' balance: 'Geïnd saldo' test_mode: 'Testmodus' + fraud_suspected: 'Vermoeden van fraude' payment_link: 'Betaallink' payment_link_help: 'Iedereen met deze link kan de bestelling betalen in het Quickpay-betaalvenster — stuur hem naar de klant om een telefonische of factuurbestelling te innen, of na een afgebroken afrekening.' copy: 'Kopiëren' diff --git a/src/Resources/translations/messages.no.yaml b/src/Resources/translations/messages.no.yaml index 2263ecb..b5e7078 100644 --- a/src/Resources/translations/messages.no.yaml +++ b/src/Resources/translations/messages.no.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Ingen operasjoner ennå' balance: 'Trukket saldo' test_mode: 'Testmodus' + fraud_suspected: 'Mistanke om svindel' payment_link: 'Betalingslenke' payment_link_help: 'Alle med denne lenken kan betale ordren i Quickpays betalingsvindu — send den til kunden for å kreve inn betaling for en telefon- eller fakturaordre, eller etter en avbrutt kasse.' copy: 'Kopier' diff --git a/src/Resources/translations/messages.pl.yaml b/src/Resources/translations/messages.pl.yaml index 9f923cc..575154c 100644 --- a/src/Resources/translations/messages.pl.yaml +++ b/src/Resources/translations/messages.pl.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Brak operacji' balance: 'Pobrane saldo' test_mode: 'Tryb testowy' + fraud_suspected: 'Podejrzenie oszustwa' payment_link: 'Link do płatności' payment_link_help: 'Każdy, kto ma ten link, może opłacić zamówienie w oknie płatności Quickpay — wyślij go klientowi, aby pobrać płatność za zamówienie telefoniczne lub na fakturę, albo po porzuconym zamówieniu.' copy: 'Kopiuj' diff --git a/src/Resources/translations/messages.pt.yaml b/src/Resources/translations/messages.pt.yaml index b0bba5a..495df6e 100644 --- a/src/Resources/translations/messages.pt.yaml +++ b/src/Resources/translations/messages.pt.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Ainda sem operações' balance: 'Saldo capturado' test_mode: 'Modo de teste' + fraud_suspected: 'Suspeita de fraude' payment_link: 'Link de pagamento' payment_link_help: 'Qualquer pessoa com este link pode pagar a encomenda na janela de pagamento da Quickpay — envie-o ao cliente para cobrar uma encomenda por telefone ou fatura, ou após um checkout abandonado.' copy: 'Copiar' diff --git a/src/Resources/translations/messages.ro.yaml b/src/Resources/translations/messages.ro.yaml index ae142bf..c0ead03 100644 --- a/src/Resources/translations/messages.ro.yaml +++ b/src/Resources/translations/messages.ro.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Nicio operațiune încă' balance: 'Sold capturat' test_mode: 'Mod de test' + fraud_suspected: 'Suspiciune de fraudă' payment_link: 'Link de plată' payment_link_help: 'Oricine are acest link poate plăti comanda în fereastra de plată Quickpay — trimiteți-l clientului pentru a încasa o comandă telefonică sau pe factură, ori după o finalizare abandonată.' copy: 'Copiază' diff --git a/src/Resources/translations/messages.sv.yaml b/src/Resources/translations/messages.sv.yaml index 1a59759..dc9c98d 100644 --- a/src/Resources/translations/messages.sv.yaml +++ b/src/Resources/translations/messages.sv.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Inga operationer ännu' balance: 'Debiterat saldo' test_mode: 'Testläge' + fraud_suspected: 'Misstänkt bedrägeri' payment_link: 'Betalningslänk' payment_link_help: 'Alla med den här länken kan betala ordern i Quickpays betalningsfönster — skicka den till kunden för att ta betalt för en telefon- eller fakturaorder, eller efter en avbruten kassa.' copy: 'Kopiera' diff --git a/src/Resources/translations/messages.uk.yaml b/src/Resources/translations/messages.uk.yaml index 4477ccf..efa45dd 100644 --- a/src/Resources/translations/messages.uk.yaml +++ b/src/Resources/translations/messages.uk.yaml @@ -36,6 +36,7 @@ setono_sylius_quickpay: no_operations: 'Операцій поки немає' balance: 'Списаний баланс' test_mode: 'Тестовий режим' + fraud_suspected: 'Підозра на шахрайство' payment_link: 'Посилання для оплати' payment_link_help: 'Будь-хто з цим посиланням може оплатити замовлення у вікні оплати Quickpay — надішліть його клієнту, щоб отримати оплату за телефонне замовлення чи замовлення за рахунком, або після незавершеного оформлення.' copy: 'Копіювати' diff --git a/src/Resources/views/admin/order/show/payment/_operations.html.twig b/src/Resources/views/admin/order/show/payment/_operations.html.twig index 1076bdc..f9dbc64 100644 --- a/src/Resources/views/admin/order/show/payment/_operations.html.twig +++ b/src/Resources/views/admin/order/show/payment/_operations.html.twig @@ -38,4 +38,7 @@ {% if quickpay_payment.testMode %} {{ 'setono_sylius_quickpay.ui.test_mode'|trans }} {% endif %} + {% if quickpay_payment.metadata is not null and quickpay_payment.metadata.fraudSuspected %} + {{ 'setono_sylius_quickpay.ui.fraud_suspected'|trans }} + {% endif %} diff --git a/src/StateMachine/PaymentProcessor.php b/src/StateMachine/PaymentProcessor.php index 16abb88..c7fcfcd 100644 --- a/src/StateMachine/PaymentProcessor.php +++ b/src/StateMachine/PaymentProcessor.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerAwareTrait; use Setono\Quickpay\Exception\QuickpayException; +use Setono\SyliusQuickpayPlugin\Fraud\FraudCheckerInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Payment\PaymentTransitions; @@ -23,9 +24,11 @@ final class PaymentProcessor implements PaymentProcessorInterface, LoggerAwareIn public function __construct( private readonly Payum $payum, + private readonly FraudCheckerInterface $fraudChecker, private readonly bool $captureEnabled, private readonly bool $refundEnabled, private readonly bool $cancelEnabled, + private readonly bool $blockCaptureOnSuspectedFraud, ) { } @@ -63,6 +66,18 @@ public function __invoke(PaymentInterface $payment, string $transition): void return; } + // Opt-in guard: leave a fraud suspected payment for manual review instead of taking + // the money automatically. The transition itself proceeds — the merchant captures + // (or cancels) in the Quickpay manager after reviewing + if ($this->blockCaptureOnSuspectedFraud && $this->fraudChecker->isFraudSuspected($payment)) { + $this->logger?->warning('Skipped the automatic capture: Quickpay reports the payment as fraud suspected. Review the payment and capture or cancel it manually', [ + 'quickpayPaymentId' => $quickpayPaymentId, + 'paymentId' => $payment->getId(), + ]); + + return; + } + $gateway->execute(new Capture($payment)); break; diff --git a/tests/Command/ReconcilePaymentsCommandTest.php b/tests/Command/ReconcilePaymentsCommandTest.php index 8263f99..89042ca 100644 --- a/tests/Command/ReconcilePaymentsCommandTest.php +++ b/tests/Command/ReconcilePaymentsCommandTest.php @@ -14,13 +14,18 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Setono\Payum\Quickpay\QuickpayGatewayFactory; +use Setono\Quickpay\Client\Client; use Setono\Quickpay\Exception\ValidationException; use Setono\SyliusQuickpayPlugin\Command\ReconcilePaymentsCommand; use Setono\SyliusQuickpayPlugin\Provider\PendingPaymentProviderInterface; +use Setono\SyliusQuickpayPlugin\Quickpay\ClientFactoryInterface; +use Setono\SyliusQuickpayPlugin\Tests\Quickpay\FixedResponseHttpClient; use Sylius\Abstraction\StateMachine\StateMachineInterface; use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; +use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; @@ -41,6 +46,12 @@ final class ReconcilePaymentsCommandTest extends TestCase /** @var ObjectProphecy */ private ObjectProphecy $stateMachine; + /** @var ObjectProphecy */ + private ObjectProphecy $clientFactory; + + /** @var ObjectProphecy> */ + private ObjectProphecy $gatewayConfigRepository; + /** @var ObjectProphecy */ private ObjectProphecy $entityManager; @@ -53,6 +64,11 @@ protected function setUp(): void $this->payum = $this->prophesize(Payum::class); $this->gateway = $this->prophesize(GatewayInterface::class); $this->stateMachine = $this->prophesize(StateMachineInterface::class); + $this->clientFactory = $this->prophesize(ClientFactoryInterface::class); + + /** @var ObjectProphecy> $gatewayConfigRepository */ + $gatewayConfigRepository = $this->prophesize(RepositoryInterface::class); + $this->gatewayConfigRepository = $gatewayConfigRepository; $this->entityManager = $this->prophesize(EntityManagerInterface::class); $this->managerRegistry = $this->prophesize(ManagerRegistry::class); $this->managerRegistry->getManagerForClass(Argument::type('string'))->willReturn($this->entityManager); @@ -227,6 +243,45 @@ public function it_rejects_an_unparsable_since_option(): void self::assertStringContainsString('Cannot parse "not-a-period"', $tester->getDisplay()); } + /** + * @test + */ + public function it_reports_fraud_suspected_payments_without_touching_any_payment(): void + { + $gatewayConfig = $this->prophesize(GatewayConfigInterface::class); + $gatewayConfig->getConfig()->willReturn(['api_key' => 'the-api-key']); + $gatewayConfig->getGatewayName()->willReturn('quickpay_credit_card'); + + $this->gatewayConfigRepository + ->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) + ->willReturn([$gatewayConfig->reveal()]) + ; + + $response = new Response(200, ['Content-Type' => 'application/json'], (string) json_encode([[ + 'id' => 501, + 'order_id' => 'qp_000000123', + 'currency' => 'DKK', + 'state' => 'new', + 'merchant_id' => 1, + 'test_mode' => true, + 'metadata' => ['fraud_suspected' => true], + ]])); + + $this->clientFactory + ->create('the-api-key') + ->willReturn(new Client('the-api-key', new FixedResponseHttpClient($response))) + ; + + $this->pendingPaymentProvider->findPending(Argument::cetera())->shouldNotBeCalled(); + $this->stateMachine->apply(Argument::cetera())->shouldNotBeCalled(); + + $tester = $this->executeCommand(['--fraud-suspected' => true]); + + self::assertSame(Command::SUCCESS, $tester->getStatusCode()); + self::assertStringContainsString('qp_000000123', $tester->getDisplay()); + self::assertStringContainsString('1 payment(s) flagged as fraud suspected', $tester->getDisplay()); + } + /** * @param array $input */ @@ -237,6 +292,8 @@ private function executeCommand(array $input): CommandTester $this->pendingPaymentProvider->reveal(), $this->payum->reveal(), $this->stateMachine->reveal(), + $this->clientFactory->reveal(), + $this->gatewayConfigRepository->reveal(), $this->managerRegistry->reveal(), )); diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index b8e1343..1b7e8e6 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -28,6 +28,9 @@ public function it_has_sensible_defaults(): void 'refund' => true, 'cancel' => true, ], + 'fraud' => [ + 'block_capture' => false, + ], 'checkout' => [ 'payment_method_logos' => [], 'creditcard_brands' => ['visa', 'mastercard'], @@ -49,6 +52,32 @@ public function it_allows_disabling_individual_operations(): void 'refund' => true, 'cancel' => false, ], + 'fraud' => [ + 'block_capture' => false, + ], + 'checkout' => [ + 'payment_method_logos' => [], + 'creditcard_brands' => ['visa', 'mastercard'], + ], + ]); + } + + /** + * @test + */ + public function it_allows_enabling_the_capture_fraud_guard(): void + { + $this->assertProcessedConfigurationEquals([ + ['fraud' => ['block_capture' => true]], + ], [ + 'operations' => [ + 'capture' => true, + 'refund' => true, + 'cancel' => true, + ], + 'fraud' => [ + 'block_capture' => true, + ], 'checkout' => [ 'payment_method_logos' => [], 'creditcard_brands' => ['visa', 'mastercard'], @@ -70,6 +99,9 @@ public function it_keeps_payment_method_tokens_as_configured(): void 'refund' => true, 'cancel' => true, ], + 'fraud' => [ + 'block_capture' => false, + ], 'checkout' => [ 'payment_method_logos' => ['mobilepay' => 'build/images/mobilepay.svg', 'apple-pay' => null], 'creditcard_brands' => ['dankort', 'visa'], diff --git a/tests/DependencyInjection/SetonoSyliusQuickpayExtensionTest.php b/tests/DependencyInjection/SetonoSyliusQuickpayExtensionTest.php index 69c48e6..a6fcd3c 100644 --- a/tests/DependencyInjection/SetonoSyliusQuickpayExtensionTest.php +++ b/tests/DependencyInjection/SetonoSyliusQuickpayExtensionTest.php @@ -28,6 +28,7 @@ public function it_sets_the_operation_toggle_parameters(): void $this->assertContainerBuilderHasParameter('setono_sylius_quickpay.operations.capture', true); $this->assertContainerBuilderHasParameter('setono_sylius_quickpay.operations.refund', true); $this->assertContainerBuilderHasParameter('setono_sylius_quickpay.operations.cancel', true); + $this->assertContainerBuilderHasParameter('setono_sylius_quickpay.fraud.block_capture', false); } /** diff --git a/tests/Fraud/FraudCheckerTest.php b/tests/Fraud/FraudCheckerTest.php new file mode 100644 index 0000000..8dc8147 --- /dev/null +++ b/tests/Fraud/FraudCheckerTest.php @@ -0,0 +1,141 @@ +createClientFactory($this->paymentResponse(['fraud_suspected' => true]))); + + self::assertTrue($checker->isFraudSuspected($this->createPayment())); + } + + /** + * @test + */ + public function it_reports_clean_when_quickpay_does_not_flag_the_payment(): void + { + $checker = new FraudChecker($this->createClientFactory($this->paymentResponse(['fraud_suspected' => false]))); + + self::assertFalse($checker->isFraudSuspected($this->createPayment())); + } + + /** + * @test + */ + public function it_reports_clean_when_the_payment_carries_no_fraud_metadata(): void + { + $checker = new FraudChecker($this->createClientFactory($this->paymentResponse(null))); + + self::assertFalse($checker->isFraudSuspected($this->createPayment())); + } + + /** + * @test + */ + public function it_reports_clean_when_the_payment_has_no_quickpay_payment_id(): void + { + $clientFactory = $this->prophesize(ClientFactoryInterface::class); + $clientFactory->create(Argument::any())->shouldNotBeCalled(); + + $checker = new FraudChecker($clientFactory->reveal()); + + self::assertFalse($checker->isFraudSuspected($this->createPayment(details: []))); + } + + /** + * @test + */ + public function it_reports_clean_when_the_gateway_carries_no_api_key(): void + { + $clientFactory = $this->prophesize(ClientFactoryInterface::class); + $clientFactory->create(Argument::any())->shouldNotBeCalled(); + + $checker = new FraudChecker($clientFactory->reveal()); + + self::assertFalse($checker->isFraudSuspected($this->createPayment(gatewayConfig: []))); + } + + /** + * @test + */ + public function it_fails_open_when_quickpay_cannot_be_reached(): void + { + $checker = new FraudChecker($this->createClientFactory(new Response(500, [], '{"message": "boom"}'))); + + self::assertFalse($checker->isFraudSuspected($this->createPayment())); + } + + private function createClientFactory(Response $response): ClientFactoryInterface + { + $clientFactory = $this->prophesize(ClientFactoryInterface::class); + $clientFactory + ->create('the-api-key') + ->willReturn(new Client('the-api-key', new FixedResponseHttpClient($response))) + ; + + return $clientFactory->reveal(); + } + + /** + * @param array{fraud_suspected: bool}|null $metadata + */ + private function paymentResponse(?array $metadata): Response + { + $payment = [ + 'id' => 501, + 'order_id' => 'qp_000000123', + 'currency' => 'DKK', + 'state' => 'new', + 'merchant_id' => 1, + ]; + + if (null !== $metadata) { + $payment['metadata'] = $metadata; + } + + return new Response(200, ['Content-Type' => 'application/json'], (string) json_encode($payment)); + } + + /** + * @param array $details + * @param array $gatewayConfig + */ + private function createPayment( + array $details = ['quickpayPaymentId' => 501], + array $gatewayConfig = ['api_key' => 'the-api-key'], + ): PaymentInterface { + $config = $this->prophesize(GatewayConfigInterface::class); + $config->getConfig()->willReturn($gatewayConfig); + + $method = $this->prophesize(PaymentMethodInterface::class); + $method->getGatewayConfig()->willReturn($config->reveal()); + + $payment = $this->prophesize(PaymentInterface::class); + $payment->getDetails()->willReturn($details); + $payment->getMethod()->willReturn($method->reveal()); + $payment->getId()->willReturn(1); + + return $payment->reveal(); + } +} diff --git a/tests/Quickpay/FixedResponseHttpClient.php b/tests/Quickpay/FixedResponseHttpClient.php new file mode 100644 index 0000000..8baed6d --- /dev/null +++ b/tests/Quickpay/FixedResponseHttpClient.php @@ -0,0 +1,25 @@ +response; + } +} diff --git a/tests/StateMachine/PaymentProcessorTest.php b/tests/StateMachine/PaymentProcessorTest.php index 9f903f4..4c7eb2c 100644 --- a/tests/StateMachine/PaymentProcessorTest.php +++ b/tests/StateMachine/PaymentProcessorTest.php @@ -17,6 +17,7 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Setono\Quickpay\Exception\ValidationException; +use Setono\SyliusQuickpayPlugin\Fraud\FraudCheckerInterface; use Setono\SyliusQuickpayPlugin\StateMachine\PaymentProcessor; use Sylius\Component\Core\Model\PaymentInterface; use Sylius\Component\Core\Model\PaymentMethodInterface; @@ -39,7 +40,7 @@ protected function setUp(): void */ public function it_captures_the_quickpay_payment_when_the_payment_is_completed(): void { - $processor = new PaymentProcessor($this->createPayum($this->createGateway()), true, true, true); + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); self::assertInstanceOf(GetHumanStatus::class, $this->executedRequests[0] ?? null); @@ -57,7 +58,7 @@ public function it_skips_capture_when_the_payment_is_already_captured(): void } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); self::assertNotContainsInstanceOf(Capture::class, $this->executedRequests); @@ -74,7 +75,7 @@ public function it_skips_refund_when_the_payment_is_already_refunded(): void } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_REFUND); self::assertNotContainsInstanceOf(Refund::class, $this->executedRequests); @@ -85,7 +86,7 @@ public function it_skips_refund_when_the_payment_is_already_refunded(): void */ public function it_refunds_the_quickpay_payment_when_the_payment_is_refunded(): void { - $processor = new PaymentProcessor($this->createPayum($this->createGateway()), true, true, true); + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_REFUND); self::assertInstanceOf(GetHumanStatus::class, $this->executedRequests[0] ?? null); @@ -97,7 +98,7 @@ public function it_refunds_the_quickpay_payment_when_the_payment_is_refunded(): */ public function it_cancels_the_quickpay_payment_when_the_payment_is_cancelled(): void { - $processor = new PaymentProcessor($this->createPayum($this->createGateway()), true, true, true); + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_CANCEL); self::assertInstanceOf(GetHumanStatus::class, $this->executedRequests[0] ?? null); @@ -115,7 +116,7 @@ public function it_skips_cancel_when_the_payment_is_already_cancelled(): void } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_CANCEL); self::assertNotContainsInstanceOf(Cancel::class, $this->executedRequests); @@ -132,7 +133,7 @@ public function it_does_not_block_the_cancel_transition_when_the_gateway_fails() } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_CANCEL); // Reaching this point means the exception was caught and the transition can proceed @@ -150,7 +151,7 @@ public function it_does_not_block_the_cancel_transition_on_sdk_exceptions(): voi } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_CANCEL); $this->addToAssertionCount(1); @@ -167,7 +168,7 @@ public function it_propagates_gateway_failures_on_the_complete_transition(): voi } }); - $processor = new PaymentProcessor($this->createPayum($gateway), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway), $this->createFraudChecker(), true, true, true, false); $this->expectException(HttpException::class); $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); @@ -184,7 +185,7 @@ public function it_does_nothing_when_the_payment_has_no_quickpay_payment_id(): v $payment = $this->prophesize(PaymentInterface::class); $payment->getDetails()->willReturn([]); - $processor = new PaymentProcessor($this->createPayum($gateway->reveal()), true, true, true); + $processor = new PaymentProcessor($this->createPayum($gateway->reveal()), $this->createFraudChecker(), true, true, true, false); $processor($payment->reveal(), PaymentTransitions::TRANSITION_CANCEL); } @@ -196,10 +197,44 @@ public function it_does_nothing_when_the_operation_is_disabled(): void $gateway = $this->prophesize(GatewayInterface::class); $gateway->execute(Argument::any())->shouldNotBeCalled(); - $processor = new PaymentProcessor($this->createPayum($gateway->reveal()), true, true, false); + $processor = new PaymentProcessor($this->createPayum($gateway->reveal()), $this->createFraudChecker(), true, true, false, false); $processor($this->createPayment(), PaymentTransitions::TRANSITION_CANCEL); } + /** + * @test + */ + public function it_skips_capture_when_the_fraud_guard_is_enabled_and_fraud_is_suspected(): void + { + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(true), true, true, true, true); + $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); + + self::assertNotContainsInstanceOf(Capture::class, $this->executedRequests); + } + + /** + * @test + */ + public function it_captures_when_the_fraud_guard_is_enabled_and_no_fraud_is_suspected(): void + { + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(false), true, true, true, true); + $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); + + self::assertInstanceOf(Capture::class, $this->executedRequests[1] ?? null); + } + + /** + * @test + */ + public function it_does_not_consult_the_fraud_checker_when_the_guard_is_disabled(): void + { + // createFraudChecker() without an answer prophesies that the checker is never consulted + $processor = new PaymentProcessor($this->createPayum($this->createGateway()), $this->createFraudChecker(), true, true, true, false); + $processor($this->createPayment(), PaymentTransitions::TRANSITION_COMPLETE); + + self::assertInstanceOf(Capture::class, $this->executedRequests[1] ?? null); + } + /** * @param class-string $class * @param list $objects @@ -231,6 +266,22 @@ private function createGateway(?callable $handler = null): GatewayInterface return $gateway->reveal(); } + /** + * @param bool|null $suspected null prophesies that the checker is never consulted + */ + private function createFraudChecker(?bool $suspected = null): FraudCheckerInterface + { + $fraudChecker = $this->prophesize(FraudCheckerInterface::class); + + if (null === $suspected) { + $fraudChecker->isFraudSuspected(Argument::any())->shouldNotBeCalled(); + } else { + $fraudChecker->isFraudSuspected(Argument::any())->willReturn($suspected); + } + + return $fraudChecker->reveal(); + } + private function createPayum(GatewayInterface $gateway): Payum { $payum = $this->prophesize(Payum::class); From 26d5aa9e5768309a8de2fbb1110d0e58430d1910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 24 Aug 2026 11:44:54 +0200 Subject: [PATCH 2/3] Cover the fraud report's error, skip, and limit branches and the checker's no-method case --- .../Command/ReconcilePaymentsCommandTest.php | 86 +++++++++++++++++++ tests/Fraud/FraudCheckerTest.php | 17 ++++ 2 files changed, 103 insertions(+) diff --git a/tests/Command/ReconcilePaymentsCommandTest.php b/tests/Command/ReconcilePaymentsCommandTest.php index 89042ca..c88163b 100644 --- a/tests/Command/ReconcilePaymentsCommandTest.php +++ b/tests/Command/ReconcilePaymentsCommandTest.php @@ -282,6 +282,92 @@ public function it_reports_fraud_suspected_payments_without_touching_any_payment self::assertStringContainsString('1 payment(s) flagged as fraud suspected', $tester->getDisplay()); } + /** + * @test + */ + public function it_caps_the_fraud_report_at_the_limit(): void + { + $gatewayConfig = $this->prophesize(GatewayConfigInterface::class); + $gatewayConfig->getConfig()->willReturn(['api_key' => 'the-api-key']); + $gatewayConfig->getGatewayName()->willReturn('quickpay_credit_card'); + + $this->gatewayConfigRepository + ->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) + ->willReturn([$gatewayConfig->reveal()]) + ; + + $payment = [ + 'id' => 501, + 'order_id' => 'qp_000000123', + 'currency' => 'DKK', + 'state' => 'new', + 'merchant_id' => 1, + 'metadata' => ['fraud_suspected' => true], + ]; + $response = new Response(200, ['Content-Type' => 'application/json'], (string) json_encode([ + $payment, + ['order_id' => 'qp_000000124', 'id' => 502] + $payment, + ])); + + $this->clientFactory + ->create('the-api-key') + ->willReturn(new Client('the-api-key', new FixedResponseHttpClient($response))) + ; + + $tester = $this->executeCommand(['--fraud-suspected' => true, '--limit' => '1']); + + self::assertSame(Command::SUCCESS, $tester->getStatusCode()); + self::assertStringContainsString('qp_000000123', $tester->getDisplay()); + self::assertStringNotContainsString('qp_000000124', $tester->getDisplay()); + } + + /** + * @test + */ + public function it_skips_gateways_without_an_api_key_in_the_fraud_report(): void + { + $gatewayConfig = $this->prophesize(GatewayConfigInterface::class); + $gatewayConfig->getConfig()->willReturn([]); + $gatewayConfig->getGatewayName()->willReturn('quickpay_credit_card'); + + $this->gatewayConfigRepository + ->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) + ->willReturn([$gatewayConfig->reveal()]) + ; + + $this->clientFactory->create(Argument::any())->shouldNotBeCalled(); + + $tester = $this->executeCommand(['--fraud-suspected' => true]); + + self::assertSame(Command::SUCCESS, $tester->getStatusCode()); + self::assertStringContainsString('no fraud suspected payments', $tester->getDisplay()); + } + + /** + * @test + */ + public function it_reports_an_error_when_a_gateway_cannot_be_queried_for_fraud(): void + { + $gatewayConfig = $this->prophesize(GatewayConfigInterface::class); + $gatewayConfig->getConfig()->willReturn(['api_key' => 'the-api-key']); + $gatewayConfig->getGatewayName()->willReturn('quickpay_credit_card'); + + $this->gatewayConfigRepository + ->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) + ->willReturn([$gatewayConfig->reveal()]) + ; + + $this->clientFactory + ->create('the-api-key') + ->willReturn(new Client('the-api-key', new FixedResponseHttpClient(new Response(500, [], '{"message": "boom"}')))) + ; + + $tester = $this->executeCommand(['--fraud-suspected' => true]); + + self::assertSame(Command::FAILURE, $tester->getStatusCode()); + self::assertStringContainsString('quickpay_credit_card', $tester->getDisplay()); + } + /** * @param array $input */ diff --git a/tests/Fraud/FraudCheckerTest.php b/tests/Fraud/FraudCheckerTest.php index 8dc8147..86e1403 100644 --- a/tests/Fraud/FraudCheckerTest.php +++ b/tests/Fraud/FraudCheckerTest.php @@ -76,6 +76,23 @@ public function it_reports_clean_when_the_gateway_carries_no_api_key(): void self::assertFalse($checker->isFraudSuspected($this->createPayment(gatewayConfig: []))); } + /** + * @test + */ + public function it_reports_clean_when_the_payment_has_no_method(): void + { + $clientFactory = $this->prophesize(ClientFactoryInterface::class); + $clientFactory->create(Argument::any())->shouldNotBeCalled(); + + $payment = $this->prophesize(PaymentInterface::class); + $payment->getDetails()->willReturn(['quickpayPaymentId' => 501]); + $payment->getMethod()->willReturn(null); + + $checker = new FraudChecker($clientFactory->reveal()); + + self::assertFalse($checker->isFraudSuspected($payment->reveal())); + } + /** * @test */ From 567dd089ae661bbfe1321661617f0bbe8869b1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 24 Aug 2026 11:56:13 +0200 Subject: [PATCH 3/3] Address review: reuse Details and extract the api key resolution - FraudChecker reads the payment id through the gateway library's Details helper; a present but unusable id lands in the same fail-open catch as an unreachable Quickpay, since either way the fraud question cannot be answered - The api_key/apikey resolution lived in three places (FraudChecker, PaymentOperationsAction, the reconcile fraud report); it is now the Quickpay\ApiKeyResolver helper, the one place that knows about the pre-2.0 option name --- src/Command/ReconcilePaymentsCommand.php | 8 +- .../Admin/PaymentOperationsAction.php | 29 ++---- src/Fraud/FraudChecker.php | 33 +++---- src/Quickpay/ApiKeyResolver.php | 40 +++++++++ tests/Fraud/FraudCheckerTest.php | 13 +++ tests/Quickpay/ApiKeyResolverTest.php | 89 +++++++++++++++++++ 6 files changed, 163 insertions(+), 49 deletions(-) create mode 100644 src/Quickpay/ApiKeyResolver.php create mode 100644 tests/Quickpay/ApiKeyResolverTest.php diff --git a/src/Command/ReconcilePaymentsCommand.php b/src/Command/ReconcilePaymentsCommand.php index ebb02e1..a278794 100644 --- a/src/Command/ReconcilePaymentsCommand.php +++ b/src/Command/ReconcilePaymentsCommand.php @@ -13,6 +13,7 @@ use Setono\Quickpay\Exception\QuickpayException; use Setono\Quickpay\Request\Payment\PaymentsQuery; use Setono\SyliusQuickpayPlugin\Provider\PendingPaymentProviderInterface; +use Setono\SyliusQuickpayPlugin\Quickpay\ApiKeyResolver; use Setono\SyliusQuickpayPlugin\Quickpay\ClientFactoryInterface; use Sylius\Abstraction\StateMachine\StateMachineInterface; use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; @@ -145,11 +146,8 @@ private function reportFraudSuspected(SymfonyStyle $io, \DateTimeImmutable $crea /** @var GatewayConfigInterface $gatewayConfig */ foreach ($this->gatewayConfigRepository->findBy(['factoryName' => QuickpayGatewayFactory::NAME]) as $gatewayConfig) { - $config = $gatewayConfig->getConfig(); - - // Configurations written by the 1.x form may still carry the old key - $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; - if (!is_string($apiKey) || '' === $apiKey) { + $apiKey = ApiKeyResolver::fromGatewayConfig($gatewayConfig->getConfig()); + if (null === $apiKey) { continue; } diff --git a/src/Controller/Admin/PaymentOperationsAction.php b/src/Controller/Admin/PaymentOperationsAction.php index a8965de..e930c70 100644 --- a/src/Controller/Admin/PaymentOperationsAction.php +++ b/src/Controller/Admin/PaymentOperationsAction.php @@ -4,10 +4,9 @@ namespace Setono\SyliusQuickpayPlugin\Controller\Admin; +use Setono\SyliusQuickpayPlugin\Quickpay\ApiKeyResolver; use Setono\SyliusQuickpayPlugin\Quickpay\ClientFactoryInterface; -use Sylius\Bundle\PayumBundle\Model\GatewayConfigInterface; use Sylius\Component\Core\Model\PaymentInterface; -use Sylius\Component\Core\Model\PaymentMethodInterface; use Sylius\Component\Core\Repository\PaymentRepositoryInterface; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -42,9 +41,14 @@ public function __invoke(int $id): Response throw new NotFoundHttpException(sprintf('Payment %d has no Quickpay payment id', $id)); } + $apiKey = ApiKeyResolver::fromPayment($payment); + if (null === $apiKey) { + throw new NotFoundHttpException('The payment\'s gateway carries no api key'); + } + try { $quickpayPayment = $this->clientFactory - ->create(self::resolveApiKey($payment)) + ->create($apiKey) ->payments() ->getById((int) $quickpayPaymentId) ; @@ -59,23 +63,4 @@ public function __invoke(int $id): Response 'quickpay_payment' => $quickpayPayment, ])); } - - private static function resolveApiKey(PaymentInterface $payment): string - { - /** @var PaymentMethodInterface $method */ - $method = $payment->getMethod(); - - /** @var GatewayConfigInterface $gatewayConfig */ - $gatewayConfig = $method->getGatewayConfig(); - - $config = $gatewayConfig->getConfig(); - - // Configurations written by the 1.x form may still carry the old key - $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; - if (!is_string($apiKey) || '' === $apiKey) { - throw new NotFoundHttpException('The payment\'s gateway carries no api key'); - } - - return $apiKey; - } } diff --git a/src/Fraud/FraudChecker.php b/src/Fraud/FraudChecker.php index 1217b29..aa4d8d3 100644 --- a/src/Fraud/FraudChecker.php +++ b/src/Fraud/FraudChecker.php @@ -6,9 +6,10 @@ use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerAwareTrait; +use Setono\Payum\Quickpay\Details; +use Setono\SyliusQuickpayPlugin\Quickpay\ApiKeyResolver; use Setono\SyliusQuickpayPlugin\Quickpay\ClientFactoryInterface; use Sylius\Component\Core\Model\PaymentInterface; -use Sylius\Component\Core\Model\PaymentMethodInterface; final class FraudChecker implements FraudCheckerInterface, LoggerAwareInterface { @@ -20,22 +21,25 @@ public function __construct(private readonly ClientFactoryInterface $clientFacto public function isFraudSuspected(PaymentInterface $payment): bool { - $quickpayPaymentId = $payment->getDetails()['quickpayPaymentId'] ?? null; - if (!is_numeric($quickpayPaymentId)) { + $details = new \ArrayObject($payment->getDetails()); + if (!Details::hasPaymentId($details)) { return false; } - $apiKey = self::resolveApiKey($payment); + $apiKey = ApiKeyResolver::fromPayment($payment); if (null === $apiKey) { return false; } try { - $quickpayPayment = $this->clientFactory->create($apiKey)->payments()->getById((int) $quickpayPaymentId); + // Details::paymentId() throws for a present but unusable id, which lands in the same + // fail-open catch as an unreachable Quickpay: the question cannot be answered + $quickpayPaymentId = Details::paymentId($details); + + $quickpayPayment = $this->clientFactory->create($apiKey)->payments()->getById($quickpayPaymentId); } catch (\Throwable $e) { - // An unreachable Quickpay must not block the payment flow: report the payment as clean $this->logger?->warning(sprintf('Could not check the Quickpay payment for suspected fraud: %s', $e->getMessage()), [ - 'quickpayPaymentId' => (int) $quickpayPaymentId, + 'quickpayPaymentId' => $details['quickpayPaymentId'] ?? null, 'paymentId' => $payment->getId(), ]); @@ -44,19 +48,4 @@ public function isFraudSuspected(PaymentInterface $payment): bool return true === $quickpayPayment->metadata?->fraudSuspected; } - - private static function resolveApiKey(PaymentInterface $payment): ?string - { - $method = $payment->getMethod(); - if (!$method instanceof PaymentMethodInterface) { - return null; - } - - $config = $method->getGatewayConfig()?->getConfig() ?? []; - - // Configurations written by the 1.x form may still carry the old key - $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; - - return is_string($apiKey) && '' !== $apiKey ? $apiKey : null; - } } diff --git a/src/Quickpay/ApiKeyResolver.php b/src/Quickpay/ApiKeyResolver.php new file mode 100644 index 0000000..d901164 --- /dev/null +++ b/src/Quickpay/ApiKeyResolver.php @@ -0,0 +1,40 @@ + $config + */ + public static function fromGatewayConfig(array $config): ?string + { + $apiKey = $config['api_key'] ?? $config['apikey'] ?? null; + + return is_string($apiKey) && '' !== $apiKey ? $apiKey : null; + } + + public static function fromPayment(PaymentInterface $payment): ?string + { + $method = $payment->getMethod(); + if (!$method instanceof PaymentMethodInterface) { + return null; + } + + return self::fromGatewayConfig($method->getGatewayConfig()?->getConfig() ?? []); + } +} diff --git a/tests/Fraud/FraudCheckerTest.php b/tests/Fraud/FraudCheckerTest.php index 86e1403..1e07da9 100644 --- a/tests/Fraud/FraudCheckerTest.php +++ b/tests/Fraud/FraudCheckerTest.php @@ -93,6 +93,19 @@ public function it_reports_clean_when_the_payment_has_no_method(): void self::assertFalse($checker->isFraudSuspected($payment->reveal())); } + /** + * @test + */ + public function it_fails_open_when_the_details_carry_an_unusable_quickpay_payment_id(): void + { + $clientFactory = $this->prophesize(ClientFactoryInterface::class); + $clientFactory->create(Argument::any())->shouldNotBeCalled(); + + $checker = new FraudChecker($clientFactory->reveal()); + + self::assertFalse($checker->isFraudSuspected($this->createPayment(details: ['quickpayPaymentId' => 'foo']))); + } + /** * @test */ diff --git a/tests/Quickpay/ApiKeyResolverTest.php b/tests/Quickpay/ApiKeyResolverTest.php new file mode 100644 index 0000000..7af5894 --- /dev/null +++ b/tests/Quickpay/ApiKeyResolverTest.php @@ -0,0 +1,89 @@ + $config + */ + public function it_resolves_the_api_key_from_a_gateway_config(array $config, ?string $expected): void + { + self::assertSame($expected, ApiKeyResolver::fromGatewayConfig($config)); + } + + /** + * @return iterable, string|null}> + */ + public static function gatewayConfigProvider(): iterable + { + yield 'the api_key option' => [['api_key' => 'the-api-key'], 'the-api-key']; + + yield 'the pre-2.0 apikey option' => [['apikey' => 'the-old-api-key'], 'the-old-api-key']; + + yield 'api_key wins over the pre-2.0 option' => [['api_key' => 'new', 'apikey' => 'old'], 'new']; + + yield 'no key stored' => [[], null]; + + yield 'an empty key' => [['api_key' => ''], null]; + + yield 'a non-string key' => [['api_key' => 123], null]; + } + + /** + * @test + */ + public function it_resolves_the_api_key_from_a_payment(): void + { + $config = $this->prophesize(GatewayConfigInterface::class); + $config->getConfig()->willReturn(['api_key' => 'the-api-key']); + + $method = $this->prophesize(PaymentMethodInterface::class); + $method->getGatewayConfig()->willReturn($config->reveal()); + + $payment = $this->prophesize(PaymentInterface::class); + $payment->getMethod()->willReturn($method->reveal()); + + self::assertSame('the-api-key', ApiKeyResolver::fromPayment($payment->reveal())); + } + + /** + * @test + */ + public function it_resolves_nothing_from_a_payment_without_a_method(): void + { + $payment = $this->prophesize(PaymentInterface::class); + $payment->getMethod()->willReturn(null); + + self::assertNull(ApiKeyResolver::fromPayment($payment->reveal())); + } + + /** + * @test + */ + public function it_resolves_nothing_from_a_payment_without_a_gateway_config(): void + { + $method = $this->prophesize(PaymentMethodInterface::class); + $method->getGatewayConfig()->willReturn(null); + + $payment = $this->prophesize(PaymentInterface::class); + $payment->getMethod()->willReturn($method->reveal()); + + self::assertNull(ApiKeyResolver::fromPayment($payment->reveal())); + } +}