feat: add PaymentController v2 architecture#192
Conversation
|
Seems to be nice work, better than me, tests etc. will test it on next update |
|
Using Stripe with PaymentController (before ERPNext integration) The ERPNext Payment Request doctype doesn't yet support the new PaymentController architecture. A small PR (~50 lines) is needed there to detect v2 gateways and call PaymentController.initiate(). Until then, you can still use and test this implementation by linking directly to the checkout page. Direct Usage Generate payment URLs in this format: Or programmatically: from frappe.utils import get_url params = { Security Note The URL parameters are validated server-side against the reference document. If a reference_doctype and reference_docname are provided, the amount and currency are fetched from that document directly, preventing URL tampering. Webhooks Configure your Stripe webhook to point to: Events: payment_intent.succeeded, payment_intent.payment_failed, payment_intent.canceled |
Add support for the new PaymentController architecture from frappe/payments. When a payment gateway implements PaymentController (v2), Payment Request uses the standardized flow. Existing v1 gateways continue working unchanged. Changes: - Add _is_v2_gateway() to detect PaymentController-based gateways - Add _process_v2_gateway() for v2 payment flow - Add get_tx_data() for standardized transaction data - Branch between v1/v2 flows in before_submit() The v2 flow uses PaymentController.initiate() to create a Payment Session Log and generates a standard payment URL (/pay?s=<psl_name>) that works with the payments app's unified checkout experience. Tested with Stripe gateway - Payment Request submission creates correct Payment Session Log and payment page loads successfully. Related: frappe/payments#192
|
https://github.com/0spinboson/erpnext/tree/feat/payment-controller-v2-support < here's a companion patch to test the new gateway approach blaggacao came up with for erpnext |
@0spinboson will you submit this PR to erpnext |
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
|
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Changes: - Add _is_v2_gateway() helper to detect PaymentController-based gateways - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data Related: frappe/payments#192
e8e80ae to
9453739
Compare
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Core changes: - Add _is_v2_gateway() helper delegating to payments.utils.is_v2_gateway() - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data - Store PSL reference for debugging and reconciliation Performance optimization: - Consolidate existence check with document fetch (single DB call) Security & privacy hardening: - Whitelist only payment-relevant contact fields (first_name, last_name, email_id, email, phone) instead of exposing full Contact.as_dict() - Whitelist only payment-relevant address fields (address_line1/2, city, state, pincode, country) instead of full Address.as_dict() - Add catch-all exception handler to prevent submission failures - Log full exception server-side, show generic message to users Compatibility: - Add 'email' key alongside 'email_id' for gateway compatibility - Normalize first_name to prevent None in tx_data - Use get_request_amount() for partial payment support Test coverage: - v2 gateway detection (None, empty string, nonexistent, v1, v2) - Flow selection (v2 vs v1 mutual exclusion) - tx_data structure validation - Partial payment amount handling - PII minimization (whitelisted fields only) - Exception handling Related: frappe/payments#192
|
okay, should be done now. Reorganized code changes logically into commits. Apologies for not doing this in my own fork first. |
fc7471c to
9947cc7
Compare
Adds documentation for the new PaymentController v2 architecture in the payments app, covering: - Architecture overview and components - Integration with ERPNext Payment Request - Implementing v2 payment gateways - Stripe v2 configuration guide - API reference - Troubleshooting guide Related: frappe/erpnext#51723, frappe/payments#192
Adds support for the new PaymentController interface from frappe/payments. This enables Payment Request to work with v2 payment gateways (like the refactored Stripe integration) while maintaining full backward compatibility with existing v1 gateways. Core changes: - Add _is_v2_gateway() helper delegating to payments.utils.is_v2_gateway() - Branch before_submit to use appropriate flow (v2 vs v1) - Add _process_v2_gateway() method using PaymentController.initiate() - Add get_tx_data() to prepare standardized transaction data - Store PSL reference for debugging and reconciliation Performance optimization: - Consolidate existence check with document fetch (single DB call) Security & privacy hardening: - Whitelist only payment-relevant contact fields (first_name, last_name, email_id, email, phone) instead of exposing full Contact.as_dict() - Whitelist only payment-relevant address fields (address_line1/2, city, state, pincode, country) instead of full Address.as_dict() - Add catch-all exception handler to prevent submission failures - Log full exception server-side, show generic message to users Compatibility: - Add 'email' key alongside 'email_id' for gateway compatibility - Normalize first_name to prevent None in tx_data - Use get_request_amount() for partial payment support Test coverage: - v2 gateway detection (None, empty string, nonexistent, v1, v2) - Flow selection (v2 vs v1 mutual exclusion) - tx_data structure validation - Partial payment amount handling - PII minimization (whitelisted fields only) - Exception handling Related: frappe/payments#192
f8927b6 to
f82df10
Compare
…roceed (DRY) Removes the repeated redirect-and-raise dance from three except branches in PaymentController.proceed(). Behavior is preserved exactly: the two-arg form (psl + error) is used for FailedToInitiateFlowError and HTTPError; the one-arg form (error only) is used for the generic Exception branch. Covered by new characterization test test_proceed_redirects_on_initiation_failure.
Centralises the {"gateway_settings": …, "gateway_controller": …} JSON
encode/decode pattern into a single GatewayRef dataclass in types.py.
Replaces three hand-rolled sites in payment_session_log.py (create_log,
get_controller, select_button db_set). JSON shape is identical so
existing stored values remain readable. Tested via TestGatewayRef roundtrip.
…PE_CHECKING import
The recurring/mandate design spec and its implementation plan describe the *mandate framework* (a later feature), not the v2 PaymentController architecture this branch introduces. They belong with the mandate PR; removing them keeps this branch single-purpose. ARCHITECTURE.md and the controller-test design notes (architecture-era) are retained.
psl.lock(timeout=5) was acquired before the try/finally that releases it. On lock contention it raises frappe.DocumentLockedError (a ValidationError subclass) which process_response's outer handler does not catch, so it escaped to the caller. On the muted server-to-server (webhook) path there is no error surface, so this 500s the request and the gateway retries. Wrap the lock acquisition in its own try/except: on DocumentLockedError, reload the PSL and return its current state as a Processed instead of raising. Move the lock body (including the existing is_terminal early return) inside the try whose finally unlocks, and drop the now-redundant explicit unlock on the terminal path.
…esponse _process_response guards that flags.status_changed_to belongs to one of the flowstate categories (success/pre_authorized/processing/declined) and raised a bare ValueError otherwise. process_response's outer try/except only catches PayloadIntegrityError, PaymentControllerProcessingError and RefDocHookProcessingError, so the ValueError escaped with a traceback and nothing was returned to the frontend. Raise PaymentControllerProcessingError for the unmapped status instead, so the existing error path turns it into a clean red Processed uniformly with other processing failures.
proceed's HTTPError arm read data = frappe.flags.integration_request.json(),
but the v2 code path sets frappe.flags.integration_request_doc (the PSL) and
never the v1 frappe.flags.integration_request. So any gateway raising
HTTPError during initiation hit AttributeError on NoneType.json(), the error
handler itself errored, and the original HTTPError was masked.
Capture the exception (except HTTPError as err) and read the response body
off err.response, falling back to {} when there is no response and to the
error string on non-JSON bodies.
The Payment Session Log JSON set the status field default to "Queued", but create_log() inserts with status="Created" and the state machine only recognizes Created/Started/Initiated/Data Capture/Paid/Authorized/ Processing/Declined/Cancelled/Error/Error - RefDoc. "Queued" is not a recognized state, so any PSL created without an explicit status would be invisible to the state machine. Change the schema default to "Created" so a bare insert lands in the same initial state the code uses. Add tests asserting create_log() and a bare insert both start in "Created" and that it is a recognized non-terminal state. (status is a Data field, not Select, so there is no options list to reconcile.)
get_controller() returned frappe.get_cached_doc() for the PaymentController. A controller's __init__ sets self.state = frappe._dict(); a cached instance reused within one request (e.g. a webhook processing multiple events for the same gateway) carries stale self.state between calls. Switch the controller resolution to frappe.get_doc() so each call yields a fresh instance with a fresh state. The read-only get_cached_doc lookups for "Payment Button" are left unchanged. Add tests asserting two resolutions return distinct instances and that state mutated on one does not leak into the next.
update_tx_data() merged a raw dict of updates into the stored tx_data and persisted it without reconstructing a TxData. A type/shape-mismatched update therefore corrupted the stored JSON silently; the corruption only surfaced later when load_state() ran TxData(**json.loads(...)) and raised TypeError far from the offending write. Reconstruct and validate a TxData from the merged dict before persisting so a bad update raises TypeError at update time. The contract is unchanged: callers (the controller) pass _filter_tx_data_updates(...) output, a dict of updates. Add tests for a well-formed update round-tripping through load_state() and a malformed update raising TypeError without persisting.
clear_old_logs() filtered status == "Paid" only, so Declined/Cancelled/ Error/Error - RefDoc logs were never cleaned and grew without bound. Delete ALL terminal-state logs older than the retention window using table.status.isin(TERMINAL_STATES), reusing the existing TERMINAL_STATES dict as the single source of truth rather than hardcoding a new list. The retention window could be made site-configurable in future (out of scope here). Add a test inserting backdated terminal logs (Declined/Error/Paid) plus a recent one and asserting the old terminal ones are purged and the recent one is kept.
Mirrors the get_controller fix: PaymentController carries mutable per-flow self.state, so resolving it through get_cached_doc could bleed state across resolutions within a single request. The Payment Gateway lookup stays cached (read-only config); only the stateful controller switches to get_doc.
The Payment Button rendered its gateway settings document (the full doc,
which for real gateways holds secret_key, webhook secrets and tokens) into
the Jinja context for the gateway css/js/wrapper templates. That rendered
output is injected straight into the public /pay page, so a single
`{{ doc.secret_key }}` in any gateway template would leak credentials to
every visitor.
Add PaymentController.get_frontend_safe_context() (default: {}) as an
explicit, override-per-gateway allowlist of non-secret values. PaymentButton
now builds the template context from that projection (wrapped in frappe._dict)
instead of the raw doc, so templates referencing `doc.<field>` keep working
for whitelisted fields while secrets are simply absent.
Test: assert a fake secret_key is absent from the projection and only the
whitelisted publishable_key is present.
Frappe's website Jinja renders with autoescape OFF, and pay.html interpolated several guest-controlled / guest-influenced values as raw HTML: reference_doctype, reference_docname, the payer full_name, and loyalty_points. full_name in particular is guest-updatable through proceed()'s whitelist (payer_contact is in UPDATABLE_TX_DATA_FIELDS), so a <script> payload could be stored on the PSL and then rendered unescaped into the public /pay page — stored XSS against anyone who opens the link. Apply the | e (escape) filter to every user-influenced interpolation. The deliberately-raw gateway blocks (gateway_css/gateway_js/gateway_wrapper) are HTML-by-design and left untouched (H1 governs what data flows into them); status is a controller-defined enum value, not user input. Test: verified by inspection that each user-influenced var now carries | e (rendering the full website Jinja stack in a unit test is impractical here).
TxData.payer_contact / payer_address arrive as full document dicts (contact.as_dict() / address.as_dict()), carrying PII and bookkeeping fields we neither need nor want — email/phone plus owner, modified_by, creation, timestamps, idx, doctype and arbitrary custom fields. These are persisted in the PSL for the whole retention window and are reachable from guest-rendered template context. create_log() now projects both down to documented allowlists before persisting (contact: full_name/email_id/phone/mobile_no; address: address_line1/address_line2/city/state/country/pincode). Projection is defensive: only applied when the value is a dict, tolerant of missing keys. Test: pass payer dicts containing extra/sensitive keys (owner, secret_note, custom_internal) through create_log and assert they are stripped while the allowlisted fields survive; empty/partial dicts don't raise.
… guests Guest-facing failure messages interpolated the Error Log document directly: str(error_log) is the Error Log docname, which embeds Frappe's internal timestamp/naming scheme. That leaked internal state to anyone hitting the failure path. Add a small _error_ref() helper (in both payment_session_log.py and payment_controller.py) that exposes only the trailing 8 chars of the Error Log name as an opaque correlation code. Support can still correlate against the full server-side Error Log, but the internal naming/timestamp is no longer disclosed. Applied to: - select_button()'s guest message_log entries - _error_value() (process_response error surface) - _redirect_on_initiation_error() (the /pay redirect message); the PSL name there is already known to the user via the /pay URL, so only the Error Log ref is shortened. The full Error Log reference is still kept server-side and in the support mailto body (where the user legitimately needs it to contact support). Test: assert the guest message carries the short 8-char code and not the full Error Log docname.
select_button is @frappe.whitelist(allow_guest=True), and Frappe skips CSRF validation for guest sessions, so the endpoint could be driven cross-origin. Impact is bounded (an attacker must already know a valid ~35-bit, non-terminal PSL name, and the call only switches among already-enabled buttons matching the PSL's gateway filter — it never alters amount or reference doc), but it could still redirect a victim toward an attacker-preferred gateway. Restrict the endpoint to methods=["POST"]. This blocks the trivial cross-origin vectors (GET via link/<img>, simple form GET). The existing client uses frappe.call (POST), so no functional change. Residual risk (DONE_WITH_CONCERNS): a fully scripted cross-origin POST (fetch/XHR) remains possible if the attacker knows a valid PSL name. A proper fix is a per-session CSRF token issued in the /pay page context and required by select_button; intentionally deferred as a follow-up rather than half-building token infra on this branch. Test: assert select_button is registered with methods=["POST"] (and not GET) via frappe.allowed_http_methods_for_whitelisted_func.
The earlier M4 commit hardened only the PSL-not-found branch; the button-not-found branch still formatted the full Error Log docname (with its internal timestamp/naming) into the guest message, and embedded a stray <br>. Use the opaque _error_ref code here too, matching the other path.
_process_response did three jobs: run the gateway processor + validate the status category, map status->PSL state + build the default Processed, and invoke the optional ref-doc hook that may override the return value. Extract the third job into a private _invoke_ref_doc_hook(ref_doc, changed, ret, processed) so _process_response reads linearly and the hook path is independently readable/testable. Control flow, the RefDocHookProcessingError wrapping (with client message-log scrubbing), and the returned (possibly overridden) Processed are all unchanged. Behavior-preserving.
…state
update_gateway_specific_state and set_initiation_payload performed the
identical write: db_set({initiation_response_payload: frappe.as_json(value),
status: status}, commit=True), differing only in the semantic name of the
payload arg. Extract a private _set_initiation_state(payload, status) that
does the as_json + two-key committed db_set once; both public methods now
delegate to it, keeping their external contracts (same args, same persisted
result) unchanged.
Reconciliation note: both methods already wrapped their value in
frappe.as_json (neither expected a pre-serialized string), so there was no
as_json inconsistency to fix — the shared helper preserves that single
serialization point exactly. Behavior-preserving.
PSLName(str) and PaymentUrl(str) were empty str subclasses used purely as type-annotation markers — never instantiated (no PSLName(x) calls) nor isinstance-checked anywhere in the repo. Replace them with TypeAlias = str (project targets 3.10+). The names remain importable exactly as before (from payments.types import PSLName), so all signatures are unchanged. Precision win: callers no longer carry a surprising str-subclass identity (type(x) is str now holds), with zero runtime behavior change since the markers were annotation-only.
PaymentController.__new__ re-validated that the class declares flowstates and frontend_defaults (as SessionStates/FrontendDefaults instances) on EVERY instantiation. These are subclass-DEFINITION invariants, so move them to __init_subclass__, which runs once at class-definition (import) time. The two checks and their TypeError messages are unchanged; super().__init_subclass__ is called first. Why definition-time is correct: the invariant is about how a gateway class is declared, not about any per-instance state, so import time is the earliest and cheapest moment to catch a misdeclared controller. __init_subclass__ runs for subclasses only, so the PaymentController base (which legitimately declares neither) is never checked — confirmed it still imports. The sole concrete gateway, PaymentDemoSettings, declares both as class attributes and imports cleanly. Add TestSubclassInvariants asserting the TypeError now fires at class DEFINITION (bad subclass) and that a well-formed subclass defines without error. Behavior-preserving.
…[str, float]) list[str, float] is invalid (list takes one type arg). The field is a (label, value) pair rendered as loyalty_points[0]/[1] in pay.html, so the correct annotation is tuple[str, float] | None.
…stract intermediates
789dc7a to
50535fa
Compare
|
Quick disclosure: I work at Juspay, and the library below (hyperswitch-prism) is ours. First off, this is the right shape. The The controller standardizes the orchestration, but every concrete gateway still hand-writes the how in the abstract methods ( This is where Prism fits. It's a stateless connector library with one request/response contract across 100+ processors, so you write a single To be clear on scope, it's not a second controller and not for the existing gateways. It's additive and opt-in, aimed at the regional processors the current 8 don't reach (Mollie for EU riding on #68, then Adyen, Paystack #97, Flutterwave, dLocal). Prism's If the direction is welcome, I'm happy to put together a |
Summary
Adds a
PaymentControllerbase class that standardizes payment gateway integrations using the template method pattern. Based on the architecture from PR #53 by @blaggacao.The first gateway implementation (Stripe PaymentIntent) is on the
feat/stripe-payment-intentbranch, stacked on this one.What's included
PaymentController— abstract base class orchestratinginitiate → proceed → process_responsePayment Session Log— DocType for tracking payment state with document locking and terminal state checksPayment Button— DocType for gateway selection UI/pay— gateway-agnostic payment endpointpayments/types.py— data contracts:TxData,Initiated,Proceeded,Processed,SessionStatespayments/exceptions.py— structured error hierarchyis_v2_gateway()— utility for consumers to detect v2-compatible gatewaysdelete_custom_fieldsto use composite key lookupget_checkout_urlto route throughget_payment_gateway_controllerPayment Session Loglink field on Payment RequestThe interface currently supports the
chargeflow. Subscription, pre-authorization, and mandate flows are planned.Tests
28 Python tests covering controller lifecycle, PSL state management, PaymentButton properties, and legacy compatibility.
Breaking changes
None. Existing gateway integrations are unaffected.