From 84a51e00d2af98b70688941b39ad37fcb354dba2 Mon Sep 17 00:00:00 2001 From: Jules Nsenda Date: Mon, 28 Sep 2026 11:11:15 +0200 Subject: [PATCH 1/2] feat: add TransactionValidator and shared SubunitConverter (R2.1) Introduces the accept-side verification gate's first building block: a pure TransactionValidator checking a Paystack verify response against status, currency, bounded amount window, and live/test domain, plus a SubunitConverter shared between the send side (Setup.php) and this validator so the two can never independently drift. Not wired into any consumer yet. Verified on dev-repro that the browser's popup amount and the server's post-placement order total agree exactly, on both a same-currency and a display-currency-!=-base-currency guest checkout, before implementing the amount comparand. --- Controller/Payment/Setup.php | 16 +- Gateway/PaystackApiClient.php | 18 +- Gateway/SubunitConverter.php | 45 ++ Gateway/Validator/TransactionValidator.php | 190 ++++++++ Test/Unit/Controller/Payment/SetupTest.php | 98 +++- Test/Unit/Gateway/PaystackApiClientTest.php | 22 + Test/Unit/Gateway/SubunitConverterTest.php | 60 +++ .../Validator/TransactionValidatorTest.php | 423 ++++++++++++++++++ 8 files changed, 868 insertions(+), 4 deletions(-) create mode 100644 Gateway/SubunitConverter.php create mode 100644 Gateway/Validator/TransactionValidator.php create mode 100644 Test/Unit/Gateway/SubunitConverterTest.php create mode 100644 Test/Unit/Gateway/Validator/TransactionValidatorTest.php diff --git a/Controller/Payment/Setup.php b/Controller/Payment/Setup.php index c86cd8a..1e678b5 100644 --- a/Controller/Payment/Setup.php +++ b/Controller/Payment/Setup.php @@ -22,6 +22,8 @@ namespace Pstk\Paystack\Controller\Payment; +use Pstk\Paystack\Gateway\SubunitConverter; + class Setup extends AbstractPaystackStandard { /** @@ -60,10 +62,22 @@ protected function processAuthorization(\Magento\Sales\Model\Order $order) { ); } + // Fail closed on a missing/malformed order grand total. A non-numeric or + // zero total would silently send amount:0 to Paystack, which would let a + // customer "pay" nothing with a success response and no trace. The caller + // turns this into order history plus the failure page. + $grandTotal = $order->getGrandTotal(); + $amount = is_numeric($grandTotal) ? SubunitConverter::toSubunit($grandTotal) : 0; + if ($amount <= 0) { + throw new \Pstk\Paystack\Gateway\Exception\ApiException( + 'Cannot start a Paystack transaction: the order has no valid grand total.' + ); + } + $tranx = $this->paystackClient->initializeTransaction([ 'first_name' => $order->getCustomerFirstname(), 'last_name' => $order->getCustomerLastname(), - 'amount' => (int) round($order->getGrandTotal() * 100), // in kobo (integer, subunit) + 'amount' => $amount, // in kobo (integer, subunit) 'email' => $order->getCustomerEmail(), // unique to customers 'reference' => $order->getIncrementId(), // unique to transactions 'currency' => $currency, diff --git a/Gateway/PaystackApiClient.php b/Gateway/PaystackApiClient.php index 6a43e9f..1f43d28 100644 --- a/Gateway/PaystackApiClient.php +++ b/Gateway/PaystackApiClient.php @@ -29,7 +29,7 @@ private function getSecretKey(): string if ($this->secretKey === null) { $method = $this->paymentHelper->getMethodInstance(PaystackModel::CODE); $this->secretKey = $method->getConfigData('live_secret_key'); - if ($method->getConfigData('test_mode')) { + if ($this->isTestMode()) { $this->secretKey = $method->getConfigData('test_secret_key'); } $this->secretKey = (string) $this->secretKey; @@ -37,6 +37,22 @@ private function getSecretKey(): string return $this->secretKey; } + /** + * Whether the store is configured for Paystack test mode. The single + * source of truth for `payment/pstk_paystack/test_mode` — `getSecretKey()` + * calls this internally rather than reading the config a second time, so + * the two can never independently drift (the same drift class + * `Gateway/SubunitConverter.php` exists to prevent). + * + * @return bool + */ + public function isTestMode(): bool + { + return (bool) $this->paymentHelper + ->getMethodInstance(PaystackModel::CODE) + ->getConfigData('test_mode'); + } + /** * Initialize a transaction (Standard/Redirect flow). * diff --git a/Gateway/SubunitConverter.php b/Gateway/SubunitConverter.php new file mode 100644 index 0000000..0cc1716 --- /dev/null +++ b/Gateway/SubunitConverter.php @@ -0,0 +1,45 @@ + you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see //www.gnu.org/licenses/>. + */ + +namespace Pstk\Paystack\Gateway; + +/** + * Static and untyped on purpose: `sales_order.grand_total` arrives from PDO as + * a numeric string. Any FUTURE caller file that declares `strict_types=1` + * would get a `TypeError` calling this with a numeric string like + * `'19.9900'`, and this module's money-path callers swallow `\Throwable` — + * turning that into a silent refusal of every paid order. Untyped and + * non-strict on purpose. Shared by Controller/Payment/Setup.php (send side) + * and Gateway/Validator/TransactionValidator.php (accept side) so the two can + * never drift apart independently. + * + * `* 100` is correct for every currency Paystack supports, including + * zero-decimal currencies like XOF/RWF — verified, do not add a + * per-currency multiplier map. + */ +class SubunitConverter +{ + public static function toSubunit($amount): int + { + return (int) round(((float) $amount) * 100); + } +} diff --git a/Gateway/Validator/TransactionValidator.php b/Gateway/Validator/TransactionValidator.php new file mode 100644 index 0000000..53a1421 --- /dev/null +++ b/Gateway/Validator/TransactionValidator.php @@ -0,0 +1,190 @@ + you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see //www.gnu.org/licenses/>. + */ + +namespace Pstk\Paystack\Gateway\Validator; + +use Magento\Sales\Api\Data\OrderInterface; +use Pstk\Paystack\Gateway\SubunitConverter; + +/** + * Checks a Paystack "verify transaction" response against the Magento order + * it claims to pay for, on four axes: status, currency, amount (bounded + * window), and live/test domain. + * + * What this validator does NOT establish — `validate() === []` means the + * response passed these four checks, nothing more: + * - reference↔order binding: no binding between a Paystack reference and a + * Magento order exists anywhere in this module today; a caller that + * advances an order on `validate() === []` alone would allow one charge + * to settle two orders. That binding is a separate concern owned + * elsewhere, not here. + * - order state: a `canceled`/`closed` order passes all four axes just the + * same as a fresh one — state is a separate concern owned elsewhere. + * - replay/idempotency: a known-successful reference re-validates + * indefinitely — nothing here tracks "already consumed." + * Callers must not treat an empty array as "safe to advance this order" on + * its own; it is one input among several owned by other classes. + * + * The `message` strings in this class are log/order-history-facing only and + * must never be reflected back to an anonymous caller — there is a known + * reflection leak elsewhere in this codebase via + * `PaymentManagement::verifyPayment()`'s `\Throwable` catch; do not repeat + * that pattern with these messages. + * + * Why not `Magento\Payment\Gateway\Validator\ValidatorInterface` / + * `Result` / `ValidatorPool`: this module has no gateway-command DI wiring + * anywhere — no `ValidatorPool`/`CommandPool` in any `etc/**\/di.xml`, and + * `Model/Payment/Paystack.php` is `AbstractMethod`-based, not + * `Adapter`-based. Adopting Magento's validator interface here would fight + * the module's actual architecture for no consumer that asks for it. + */ +class TransactionValidator +{ + const STATUS_NOT_SUCCESS = 'status_not_success'; + const STATUS_AWAITING_CONFIRMATION = 'awaiting_confirmation'; + const CURRENCY_MISMATCH = 'currency_mismatch'; + const AMOUNT_MISMATCH = 'amount_mismatch'; + const AMOUNT_EXCESS = 'amount_excess'; + const ORDER_AMOUNT_INVALID = 'order_amount_invalid'; + const MODE_MISMATCH = 'mode_mismatch'; + + /** + * Tolerance above the expected subunit amount. Covers orders paid by the + * legacy `Math.ceil` build (pre-R1.2), which could send one subunit more + * than `SubunitConverter::toSubunit()` computes today for the same + * order — not unexplained slack, do not delete. Whoever registers the + * captured payment later must decide whether the verified `data.amount` + * (possibly one subunit over) or the order's own total is what gets + * recorded — this class does not decide that. + */ + const TOLERANCE_SUBUNITS = 1; + + /** + * @param object|null $verifyData Decoded `data` object from Paystack's + * verify-transaction response. Nullable and loosely typed on purpose: + * callers receive this from JSON decoding of a third-party response, + * so missing/malformed fields must fail closed, never throw. + * @param OrderInterface $order + * @param bool $testMode Whether the store is configured for Paystack test + * mode. Must come from `PaystackApiClient::isTestMode()`, not from an + * independent config read — that closes the exact drift class this + * item's SubunitConverter change was designed to prevent, where two + * independently-maintained reads of the same config value can + * silently disagree. (Today `isTestMode()` and this validator are + * both unscoped/current-store; if a future caller resolves this + * against `$order->getStoreId()` while the client stays unscoped, a + * multi-website override makes the two disagree and real payments get + * held. Not fixed here.) + * @return array{code: string, message: string}[] Empty array means valid. + */ + public function validate(?object $verifyData, OrderInterface $order, bool $testMode): array + { + $status = $verifyData->status ?? null; + + if ($status !== 'success') { + // `pending`, `ongoing` and `queued` are in-flight, not failed: bank + // transfer and USSD sit there at verify time and settle minutes + // later via the webhook (same list as Controller/Payment/Callback.php). + // Collapsing this into STATUS_NOT_SUCCESS would regress that + // distinction for any caller that replaces its own inline check + // with this validator. + if (in_array($status, ['pending', 'ongoing', 'queued'], true)) { + return [ + ['code' => self::STATUS_AWAITING_CONFIRMATION, 'message' => 'Transaction is still being confirmed.'], + ]; + } + + return [ + ['code' => self::STATUS_NOT_SUCCESS, 'message' => 'Transaction status is not success.'], + ]; + } + + $errors = []; + + $rawCurrency = $verifyData->currency ?? ''; + $verifyCurrency = is_scalar($rawCurrency) ? strtoupper((string) $rawCurrency) : ''; + $orderCurrency = strtoupper((string) $order->getOrderCurrencyCode()); + if ($verifyCurrency === '' || $orderCurrency === '' || $verifyCurrency !== $orderCurrency) { + $errors[] = ['code' => self::CURRENCY_MISMATCH, 'message' => 'Transaction currency does not match order currency.']; + } + + $grandTotal = $order->getGrandTotal(); + $expected = is_numeric($grandTotal) ? SubunitConverter::toSubunit($grandTotal) : 0; + if ($expected <= 0) { + $errors[] = ['code' => self::ORDER_AMOUNT_INVALID, 'message' => 'Order amount is missing or invalid.']; + } else { + $amount = $verifyData->amount ?? null; + if (!is_numeric($amount)) { + $errors[] = ['code' => self::AMOUNT_MISMATCH, 'message' => 'Transaction amount is missing or invalid.']; + } elseif ($amount < $expected) { + $errors[] = ['code' => self::AMOUNT_MISMATCH, 'message' => 'Transaction amount is less than the order total.']; + } elseif ($amount > $expected + self::TOLERANCE_SUBUNITS) { + $errors[] = ['code' => self::AMOUNT_EXCESS, 'message' => 'Transaction amount exceeds the order total.']; + } + } + + /** + * This does NOT catch "live storefront left in test mode" — that + * scenario's `$testMode` is true by construction, since `test_mode=1` + * selects the test secret key that produced the test-domain response; + * the two are coupled at the source. What it actually catches is a + * swapped/misconfigured secret key pair (e.g. a test key configured + * where a live key belongs, or vice versa). + * + * A caller that needs to know whether real money moved must not infer + * it from which codes this method returns — call `chargeIsReal()` + * instead, which derives that fact directly from `$verifyData`. + */ + $domain = $verifyData->domain ?? null; + $expectedDomain = $testMode ? 'test' : 'live'; + if ($domain !== $expectedDomain) { + $errors[] = ['code' => self::MODE_MISMATCH, 'message' => 'Transaction domain does not match configured mode.']; + } + + return $errors; + } + + /** + * Whether a Paystack verify response represents a real charge (money + * actually moved), independent of which — if any — codes `validate()` + * returns for the same `$verifyData`. + * + * Derivation: currency/amount/domain are only checked by `validate()` + * when `$verifyData->status === 'success'`. A 'success' status on + * Paystack's *test* domain is a sandbox charge (no real money moves); + * 'success' on the *live* domain is a real charge. So "real money moved" + * is fully determined by `status === 'success' && domain === 'live'`. + * + * This exists specifically so a future caller (e.g. R2.6, deciding + * whether to suppress a "retry" prompt) doesn't have to infer "did money + * move" from which `validate()` codes are present — `MODE_MISMATCH` alone + * does not tell you that, but this method does. + * + * @param object|null $verifyData Same decoded `data` object passed to + * `validate()`. + * @return bool + */ + public static function chargeIsReal(?object $verifyData): bool + { + return ($verifyData->status ?? null) === 'success' && ($verifyData->domain ?? null) === 'live'; + } +} diff --git a/Test/Unit/Controller/Payment/SetupTest.php b/Test/Unit/Controller/Payment/SetupTest.php index 88df6e6..05a8a5e 100644 --- a/Test/Unit/Controller/Payment/SetupTest.php +++ b/Test/Unit/Controller/Payment/SetupTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\MockObject\MockObject; use Pstk\Paystack\Controller\Payment\Setup; use Pstk\Paystack\Gateway\PaystackApiClient; +use Pstk\Paystack\Gateway\SubunitConverter; use Pstk\Paystack\Gateway\Exception\ApiException; use Pstk\Paystack\Model\Payment\Paystack; use Pstk\Paystack\Model\Ui\ConfigProvider; @@ -177,9 +178,10 @@ public function testFractionalTotalsAreSentAsIntegerSubunits(float $grandTotal, $this->paystackClient->expects($this->once()) ->method('initializeTransaction') - ->with($this->callback(function ($params) use ($expectedSubunits) { + ->with($this->callback(function ($params) use ($expectedSubunits, $grandTotal) { return $params['amount'] === $expectedSubunits - && is_int($params['amount']); + && is_int($params['amount']) + && $params['amount'] === SubunitConverter::toSubunit($grandTotal); })) ->willReturn((object) ['data' => (object) ['authorization_url' => 'https://checkout.paystack.com/abc123']]); @@ -216,6 +218,98 @@ public function testMissingOrderCurrencyIsRejectedBeforeCallingPaystack(): void $controller->execute(); } + /** + * A malformed grand total (missing/zero/non-numeric) must fail closed + * rather than silently send amount:0 to Paystack, letting a customer + * "pay" nothing with a success response and no trace. + */ + public function testMissingGrandTotalIsRejectedBeforeCallingPaystack(): void + { + $controller = $this->createController(); + + $lastOrder = $this->createMock(Order::class); + $lastOrder->method('getIncrementId')->willReturn('000000001'); + $this->checkoutSession->method('getLastRealOrder')->willReturn($lastOrder); + + $payment = $this->createMock(Payment::class); + $payment->method('getMethod')->willReturn(Paystack::CODE); + + $order = $this->createMock(Order::class); + $order->method('getPayment')->willReturn($payment); + $order->method('getStatus')->willReturn('pending'); + $order->method('getCustomerFirstname')->willReturn('John'); + $order->method('getCustomerLastname')->willReturn('Doe'); + $order->method('getGrandTotal')->willReturn(null); + $order->method('getCustomerEmail')->willReturn('john@example.com'); + $order->method('getIncrementId')->willReturn('000000001'); + $order->method('getOrderCurrencyCode')->willReturn('NGN'); + + $this->orderInterface->method('loadByIncrementId')->willReturn($order); + + $methodInstance = $this->createMock(MethodInterface::class); + $methodInstance->method('getCode')->willReturn(Paystack::CODE); + $this->paymentHelper->method('getMethodInstance')->willReturn($methodInstance); + + $store = $this->createMock(Store::class); + $store->method('getBaseUrl')->willReturn('https://example.com/'); + $this->storeManager->method('getStore')->willReturn($store); + + $this->paystackClient->expects($this->never())->method('initializeTransaction'); + + $order->expects($this->once()) + ->method('addStatusToHistory') + ->with('pending', $this->stringContains('grand total')); + + $controller->execute(); + } + + /** + * A grand total of exactly 0.00 is numeric (unlike null), so it takes the + * `is_numeric($grandTotal) ? SubunitConverter::toSubunit(...) : 0` branch + * of the guard rather than the "not numeric" else — pinning that a + * numeric-but-zero total is rejected too, not just a missing/non-numeric + * one. Note: from this test's vantage point the observable outcome + * (never call Paystack, same "grand total" history message) is identical + * to testMissingGrandTotalIsRejectedBeforeCallingPaystack; it does not by + * itself prove SubunitConverter was reached rather than short-circuited, + * only that this distinct input value produces the same fail-closed + * result. + */ + public function testZeroGrandTotalIsRejectedBeforeCallingPaystack(): void + { + $controller = $this->createController(); + $order = $this->primeOrder(0.00, 'NGN'); + $order->method('getStatus')->willReturn('pending'); + + $this->paystackClient->expects($this->never())->method('initializeTransaction'); + + $order->expects($this->once()) + ->method('addStatusToHistory') + ->with('pending', $this->stringContains('grand total')); + + $controller->execute(); + } + + /** + * A negative grand total is numeric too, and must be rejected by the same + * `$amount <= 0` guard rather than being sent to Paystack as a negative + * subunit amount. + */ + public function testNegativeGrandTotalIsRejectedBeforeCallingPaystack(): void + { + $controller = $this->createController(); + $order = $this->primeOrder(-19.99, 'NGN'); + $order->method('getStatus')->willReturn('pending'); + + $this->paystackClient->expects($this->never())->method('initializeTransaction'); + + $order->expects($this->once()) + ->method('addStatusToHistory') + ->with('pending', $this->stringContains('grand total')); + + $controller->execute(); + } + /** * Shared happy-path scaffolding for the cases above. * diff --git a/Test/Unit/Gateway/PaystackApiClientTest.php b/Test/Unit/Gateway/PaystackApiClientTest.php index 348b658..4408463 100644 --- a/Test/Unit/Gateway/PaystackApiClientTest.php +++ b/Test/Unit/Gateway/PaystackApiClientTest.php @@ -89,6 +89,28 @@ public function testValidateWebhookSignatureEmptySignatureFails(): void $this->assertFalse($this->client->validateWebhookSignature('body', '')); } + public function testIsTestModeTrueWhenConfigTestModeIsOn(): void + { + $this->paymentMethod->method('getConfigData') + ->willReturnCallback(function ($field) { + if ($field === 'test_mode') return true; + return null; + }); + + $this->assertTrue($this->client->isTestMode()); + } + + public function testIsTestModeFalseWhenConfigTestModeIsOff(): void + { + $this->paymentMethod->method('getConfigData') + ->willReturnCallback(function ($field) { + if ($field === 'test_mode') return false; + return null; + }); + + $this->assertFalse($this->client->isTestMode()); + } + public function testValidateWebhookSignatureTampered(): void { $this->paymentMethod->method('getConfigData') diff --git a/Test/Unit/Gateway/SubunitConverterTest.php b/Test/Unit/Gateway/SubunitConverterTest.php new file mode 100644 index 0000000..2d8869f --- /dev/null +++ b/Test/Unit/Gateway/SubunitConverterTest.php @@ -0,0 +1,60 @@ +assertSame(500000, SubunitConverter::toSubunit(5000.00)); + } + + /** + * @dataProvider inexactAmountProvider + */ + public function testFractionalAmountsRoundToNearestSubunit(float $amount, int $expected): void + { + $this->assertSame($expected, SubunitConverter::toSubunit($amount)); + } + + public static function inexactAmountProvider(): array + { + return [ + '19.99 -> 1999' => [19.99, 1999], + '8.21 -> 821' => [8.21, 821], + '1.10 -> 110' => [1.10, 110], + '0.29 -> 29' => [0.29, 29], + ]; + } + + public function testNumericStringDoesNotThrow(): void + { + $this->assertSame(1999, SubunitConverter::toSubunit('19.9900')); + } + + public function testZeroConvertsToZero(): void + { + $this->assertSame(0, SubunitConverter::toSubunit(0)); + } + + public function testNegativeAmountConvertsToNegativeSubunit(): void + { + $this->assertSame(-1999, SubunitConverter::toSubunit(-19.99)); + } + + /** + * Both current callers (Setup::processAuthorization and + * TransactionValidator::validate) guard with is_numeric() before calling + * this, so a non-numeric string is unreachable today. But this class's + * own docblock says it exists so the two callers can never independently + * drift — a future caller that drops the is_numeric guard must still get + * a fail-closed 0, not a silent cast surprise or a thrown TypeError. + */ + public function testNonNumericStringConvertsToZeroNotThrown(): void + { + $this->assertSame(0, SubunitConverter::toSubunit('abc')); + } +} diff --git a/Test/Unit/Gateway/Validator/TransactionValidatorTest.php b/Test/Unit/Gateway/Validator/TransactionValidatorTest.php new file mode 100644 index 0000000..47eefd6 --- /dev/null +++ b/Test/Unit/Gateway/Validator/TransactionValidatorTest.php @@ -0,0 +1,423 @@ +validator = new TransactionValidator(); + } + + /** + * @param float|string|null $grandTotal + * @param string $currencyCode + * @return MockObject|OrderInterface + */ + private function makeOrder($grandTotal, $currencyCode) + { + $order = $this->createMock(OrderInterface::class); + $order->method('getGrandTotal')->willReturn($grandTotal); + $order->method('getOrderCurrencyCode')->willReturn($currencyCode); + + return $order; + } + + private function makeVerifyData(array $overrides = []) + { + $data = (object) array_merge([ + 'status' => 'success', + 'currency' => 'NGN', + 'amount' => 10000, + 'domain' => 'test', + ], $overrides); + + return $data; + } + + public function testAllValidReturnsEmptyArray(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $result = $this->validator->validate($this->makeVerifyData(), $order, true); + + $this->assertSame([], $result); + } + + public function testDeclinedStatusShortCircuitsEvenWhenOthersWouldAlsoFail(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData([ + 'status' => 'failed', + 'currency' => 'USD', + 'amount' => 1, + 'domain' => 'live', + ]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertCount(1, $result); + $this->assertSame(TransactionValidator::STATUS_NOT_SUCCESS, $result[0]['code']); + } + + public function testCurrencyMismatchFailsIndependently(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['currency' => 'USD']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::CURRENCY_MISMATCH], array_column($result, 'code')); + } + + public function testAmountMismatchFailsIndependently(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 1]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::AMOUNT_MISMATCH], array_column($result, 'code')); + } + + public function testDomainMismatchFailsIndependently(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['domain' => 'live']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::MODE_MISMATCH], array_column($result, 'code')); + } + + public function testLiveDomainMatchesWhenTestModeIsFalse(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['domain' => 'live']); + + $result = $this->validator->validate($verifyData, $order, false); + + $this->assertSame([], $result); + } + + public function testEveryErrorHasCodeAndMessageKeys(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData([ + 'currency' => 'USD', + 'amount' => 1, + 'domain' => 'live', + ]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertNotEmpty($result); + foreach ($result as $error) { + $this->assertArrayHasKey('code', $error); + $this->assertArrayHasKey('message', $error); + } + } + + public function testAmountAtExactExpectedIsAccepted(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 10000]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertNotContains(TransactionValidator::AMOUNT_MISMATCH, $codes); + $this->assertNotContains(TransactionValidator::AMOUNT_EXCESS, $codes); + } + + public function testLegacyMathCeilOverpaymentOfOneSubunitIsAccepted(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 10001]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertNotContains(TransactionValidator::AMOUNT_MISMATCH, $codes); + $this->assertNotContains(TransactionValidator::AMOUNT_EXCESS, $codes); + } + + public function testAmountOneSubunitBelowExpectedIsRejected(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 9999]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::AMOUNT_MISMATCH, $codes); + } + + public function testAmountTwoSubunitsAboveExpectedIsRejectedAsExcess(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 10002]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::AMOUNT_EXCESS, $codes); + $this->assertNotContains(TransactionValidator::AMOUNT_MISMATCH, $codes); + } + + public function testStringGrandTotalDoesNotThrow(): void + { + $order = $this->makeOrder('19.9900', 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 1999]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([], $result); + } + + public function testUsesOrderCurrencyCodeNotBaseCurrencyCode(): void + { + $order = $this->createMock(OrderInterface::class); + $order->method('getGrandTotal')->willReturn(100.00); + $order->method('getOrderCurrencyCode')->willReturn('USD'); + $order->expects($this->never())->method('getBaseCurrencyCode'); + $order->expects($this->never())->method('getBaseGrandTotal'); + + $verifyData = $this->makeVerifyData(['currency' => 'USD']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([], $result); + } + + public function testCurrencyCaseDifferenceStillPasses(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['currency' => 'ngn']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([], $result); + } + + public function testNullVerifyDataShortCircuitsWithoutThrowing(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + + $result = $this->validator->validate(null, $order, true); + + $this->assertCount(1, $result); + $this->assertSame(TransactionValidator::STATUS_NOT_SUCCESS, $result[0]['code']); + } + + public function testMissingStatusPropertyShortCircuits(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = (object) ['currency' => 'NGN', 'amount' => 10000, 'domain' => 'test']; + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertCount(1, $result); + $this->assertSame(TransactionValidator::STATUS_NOT_SUCCESS, $result[0]['code']); + } + + public function testMissingCurrencyPropertyFails(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = (object) ['status' => 'success', 'amount' => 10000, 'domain' => 'test']; + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::CURRENCY_MISMATCH, $codes); + } + + public function testMissingAmountPropertyFails(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = (object) ['status' => 'success', 'currency' => 'NGN', 'domain' => 'test']; + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::AMOUNT_MISMATCH, $codes); + } + + public function testMissingDomainPropertyFails(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = (object) ['status' => 'success', 'currency' => 'NGN', 'amount' => 10000]; + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::MODE_MISMATCH, $codes); + } + + public function testZeroGrandTotalIsRejectedNotVacuouslyPassed(): void + { + $order = $this->makeOrder(0, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 0]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::ORDER_AMOUNT_INVALID, $codes); + } + + public function testNullGrandTotalIsRejected(): void + { + $order = $this->makeOrder(null, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 0]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::ORDER_AMOUNT_INVALID, $codes); + } + + public function testEmptyOrderCurrencyCodeIsRejected(): void + { + $order = $this->makeOrder(100.00, ''); + $verifyData = $this->makeVerifyData(); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::CURRENCY_MISMATCH, $codes); + } + + /** + * @dataProvider inFlightStatusProvider + */ + public function testInFlightStatusReturnsAwaitingConfirmationNotStatusNotSuccess(string $status): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['status' => $status]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::STATUS_AWAITING_CONFIRMATION], array_column($result, 'code')); + } + + public static function inFlightStatusProvider(): array + { + return [ + 'pending' => ['pending'], + 'ongoing' => ['ongoing'], + 'queued' => ['queued'], + ]; + } + + public function testGenuineDeclineStillReturnsStatusNotSuccess(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['status' => 'failed']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::STATUS_NOT_SUCCESS], array_column($result, 'code')); + } + + public function testMissingTransactionAmountFieldIsAmountMismatchNotOrderAmountInvalid(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => null]); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::AMOUNT_MISMATCH], array_column($result, 'code')); + } + + /** + * A genuinely garbage (non-numeric, non-null) amount must fail the + * is_numeric() check itself, distinct from the ?? null coalesce exercised + * by a missing/null amount property. + */ + public function testNonNumericStringAmountIsAmountMismatch(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => 'abc']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([TransactionValidator::AMOUNT_MISMATCH], array_column($result, 'code')); + } + + /** + * Paystack's JSON responses are decoded server-side; a numeric string + * (e.g. "10000") is valid JSON-number input in disguise and must be + * accepted via PHP's numeric-string comparison, not rejected outright. + */ + public function testNumericStringAmountMatchingExpectedIsAccepted(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['amount' => '10000']); + + $result = $this->validator->validate($verifyData, $order, true); + + $this->assertSame([], $result); + } + + public function testNonScalarCurrencyDoesNotThrowAndFailsClosed(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['currency' => new \stdClass()]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::CURRENCY_MISMATCH, $codes); + } + + public function testArrayCurrencyDoesNotThrowAndFailsClosed(): void + { + $order = $this->makeOrder(100.00, 'NGN'); + $verifyData = $this->makeVerifyData(['currency' => ['NGN']]); + + $result = $this->validator->validate($verifyData, $order, true); + + $codes = array_column($result, 'code'); + $this->assertContains(TransactionValidator::CURRENCY_MISMATCH, $codes); + } + + public function testChargeIsRealTrueOnlyWhenSuccessAndLiveDomain(): void + { + $verifyData = $this->makeVerifyData(['status' => 'success', 'domain' => 'live']); + + $this->assertTrue(TransactionValidator::chargeIsReal($verifyData)); + } + + public function testChargeIsRealFalseForSuccessOnTestDomain(): void + { + $verifyData = $this->makeVerifyData(['status' => 'success', 'domain' => 'test']); + + $this->assertFalse(TransactionValidator::chargeIsReal($verifyData)); + } + + public function testChargeIsRealFalseForNonSuccessStatus(): void + { + $verifyData = $this->makeVerifyData(['status' => 'failed', 'domain' => 'live']); + + $this->assertFalse(TransactionValidator::chargeIsReal($verifyData)); + } + + public function testChargeIsRealFalseForNullVerifyData(): void + { + $this->assertFalse(TransactionValidator::chargeIsReal(null)); + } + + public function testChargeIsRealFalseWhenDomainMissing(): void + { + $verifyData = (object) ['status' => 'success']; + + $this->assertFalse(TransactionValidator::chargeIsReal($verifyData)); + } +} From e952a5419d1be5b5e37ea9b40bc0ba167dde7ebc Mon Sep 17 00:00:00 2001 From: Jules Nsenda Date: Mon, 28 Sep 2026 12:47:36 +0200 Subject: [PATCH 2/2] docs: correct TransactionValidator's docblock on Paystack's base class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Model/Payment/Paystack.php extends DataObject implementing MethodInterface directly, not AbstractMethod — caught during the R2.2/R2.3 review panel. Co-Authored-By: Claude Sonnet 5 --- Gateway/Validator/TransactionValidator.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Gateway/Validator/TransactionValidator.php b/Gateway/Validator/TransactionValidator.php index 53a1421..16613f0 100644 --- a/Gateway/Validator/TransactionValidator.php +++ b/Gateway/Validator/TransactionValidator.php @@ -53,9 +53,11 @@ * Why not `Magento\Payment\Gateway\Validator\ValidatorInterface` / * `Result` / `ValidatorPool`: this module has no gateway-command DI wiring * anywhere — no `ValidatorPool`/`CommandPool` in any `etc/**\/di.xml`, and - * `Model/Payment/Paystack.php` is `AbstractMethod`-based, not - * `Adapter`-based. Adopting Magento's validator interface here would fight - * the module's actual architecture for no consumer that asks for it. + * `Model/Payment/Paystack.php` is a hand-rolled `MethodInterface` + * implementation (`extends DataObject implements MethodInterface`), not + * `AbstractMethod`- or `Adapter`-based. Adopting Magento's validator + * interface here would fight the module's actual architecture for no + * consumer that asks for it. */ class TransactionValidator {