Skip to content

feat: IPN handler + security hardening (shared-state mutation, secret exposure) - #12

Merged
birkof merged 12 commits into
mainfrom
feat/ipn-handler
Jun 17, 2026
Merged

birkof merged 12 commits into
mainfrom
feat/ipn-handler

Conversation

@birkof

@birkof birkof commented Jun 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

Adds an authenticated IPN handler to the bundle and hardens the payment flow (security + quality findings from a review).

Security fixes

  • HIGH-1 — shared-config mutation. Token payment URL /card4 is now derived from an immutable base per request via resolvePaymentUrl(); the shared NetopiaMobilPayConfiguration singleton is no longer mutated (previously accumulated /card4/card4 and leaked across requests).
  • HIGH-2 — secrets in container params. Removed the dead setParameter() calls that copied private_key/signature into the container parameter bag (cleartext cache dump). A test asserts no secret parameters are registered.

New feature — IPN handler (src/Notification/)

  • IpnAction enum (+ Unknown fallback), IpnResult immutable DTO (vendor Notify kept out of consumer code, money as string).
  • NetopiaMobilPayIpnHandler (+interface): decrypt() opens the RSA envelope with the merchant private key (decrypt = authenticity), threading cipher/iv for aes-256-cbc on OpenSSL 3; logs only $e->getCode() on failure (no crypto-step leak); confirmResponse()/errorResponse() build <crc> via DOMDocument (XML-escaped, non-self-closing <crc></crc>).
  • DI: NetopiaMobilPayConfiguration is now a single private shared service consumed by both the payment service and the public netopia_mobilpay.ipn_handler (autowired by interface alias).

Additional hardening

  • MEDIUM-3 — documented that the raw-card path puts the app in PCI-DSS SAQ-D scope; hosted payment page recommended (docblock + docs).
  • LOW-5 — signature config is now isRequired() (fail fast) instead of a placeholder default that defeated cannotBeEmpty().
  • LOW-6 — input validation (orderId/amount/currency) at the boundary before the swallowing try/catch; declare(strict_types=1) across all bundle classes.

Test Plan

  • Full suite green: vendor/bin/phpunit → 33 tests, 134 assertions
  • HIGH-1 regression: payment URL from base, no /card4/card4 accumulation
  • HIGH-2 regression: no secret container parameters
  • IPN round-trip: real openssl_seal → decrypt; plus empty/garbage payload throw
  • <crc></crc> ack non-self-closing; errorResponse sets type/code and XML-escapes message
  • LOW-5: missing signature fails config processing; LOW-6: invalid orderId/amount/currency rejected
  • php -l clean on all sources
  • Reviewer: confirm <crc></crc> ack format against a live Netopia sandbox before production use

…rams

HIGH-1: derive the token payment URL (/card4) from an immutable base on
each request instead of appending onto the shared configuration singleton,
which previously accumulated (/card4/card4) and leaked across requests.

HIGH-2: drop the dead setParameter() calls that copied private_key and
signature into the container parameter bag (dumped to the compiled
container cache in cleartext); configuration already flows via method calls.
@birkof birkof self-assigned this Jun 17, 2026
birkof added 11 commits June 17, 2026 12:56
… default

LOW-5: the signature node previously defaulted to a placeholder
('XXXX-...'), which defeated cannotBeEmpty() — a missing signature
silently booted with a bogus value. It is now isRequired(), so the
bundle fails fast at config processing. Also declares strict_types.
LOW-6: validate orderId/amount/currency at the boundary before the
try/catch so a specific input error is not masked by the generic
'Payment failed.' handler.
MEDIUM-3: warn (docblock + docs) that passing raw PAN/CVV through
composeCreditCardObject places the app in PCI-DSS SAQ-D scope; the
hosted payment page is the recommended flow. Also declares strict_types.
@birkof
birkof force-pushed the feat/ipn-handler branch from 38f0c7a to 90f02be Compare June 17, 2026 09:57
@birkof
birkof merged commit 54e568d into main Jun 17, 2026
3 checks passed
@birkof
birkof deleted the feat/ipn-handler branch June 17, 2026 11:01
birkof added a commit that referenced this pull request Jun 17, 2026
…egration guide (#13)

Replace the outdated README stub (which claimed Symfony 3.4/4.0 support) with a
grounded architecture overview plus Installation and Usage sections, and rewrite
src/Resources/doc/index.md as a complete integration guide: config/bundles.php
registration, required-signature config, the mandatory confirm/return routes, the
outbound payment flow (auto-submit form), and the inbound IPN handling flow.

Documents the IPN handler and payment-input/config hardening now on main (PR #12).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant