diff --git a/.claude/skills/add-payment-gateway/SKILL.md b/.claude/skills/add-payment-gateway/SKILL.md new file mode 100644 index 0000000..4f0c4c2 --- /dev/null +++ b/.claude/skills/add-payment-gateway/SKILL.md @@ -0,0 +1,142 @@ +--- +name: add-payment-gateway +description: Add a new payment provider (e.g. PayPal, Paymob, Checkout.com, Tap, HyperPay, Moyasar, Adyen) to bwh_payments as a Gateway Settings Single plus a base/live/stub service client in bwh_payments/services/NAME/, wired into the gateway contract, webhooks, refunds, tests and the storefront admin. Use this whenever someone wants to support, integrate, add, or "plug in" a new payment gateway, PSP, BNPL or hosted checkout in bwh_payments or commera, even if they only name the provider. +--- + +# Add a payment provider to bwh_payments + +A provider is three things: a **Single DocType** that holds credentials and implements +`PaymentGatewayBase`, a **service client** (`services//{base,live,stub}.py`) that talks to the +provider, and a **Payment Gateway Profile** record that makes it selectable. Read `AGENTS.md` +("The gateway contract", "Service clients", "Money and currency rules", "Security conventions") before +starting. Those rules are not repeated here. Copy structure from the closest existing gateway: + +| Provider looks like | Copy from | +|---|---| +| raw JSON HTTP API, HMAC-signed webhooks | `razorpay` | +| official Python SDK | `stripe` | +| no signed webhook, status only by polling | `telr` | +| authorise-then-capture (BNPL), token-checked webhooks | `tabby` | + +Branch in every repo you will touch, and make sure the bench's Redis is running before you run tests +(see the migrate skill, step 0). + +## 1. Pin down the provider's facts first + +Get these from the provider's API docs (ask the user for the link if you don't have one) and write them +in your first message back. Each one changes code, and guessing any of them costs real money: + +- **Checkout:** which endpoint creates a hosted page, which id identifies it for later status reads, and + whether that id is the one webhooks carry. Tabby's checkout id is not; its payment id is. +- **Amount format:** integer minor units (Stripe, Razorpay), a major-unit decimal string at the + currency's precision (Tabby, Telr), or something else. Find out how 3-decimal (KWD, BHD) and 0-decimal + (JPY) currencies are handled. +- **Supported currencies and markets.** +- **Status vocabulary,** and which statuses mean money is actually taken. +- **Capture model:** automatic, manual, or per-account. If it can't be known from the API, add a + `treat_authorised_as_paid` style switch that defaults to the safe side, as Razorpay and Telr do. +- **Webhook verification:** an HMAC over the raw body (with which secret), a static token, IP allowlists, + or nothing. Also find which header carries an event id; derive a stable one if there is none, as Tabby + does. +- **Refunds:** what you refund against (session, payment, transaction), whether partial refunds are + allowed, what an accepted refund looks like, and whether the refund id comes back. +- **Errors:** the error body shape, and whether failures come back as HTTP 200 (Telr does). + +## 2. Create the settings Single + +Do not `mkdir` doctype folders. With `developer_mode` on, create the DocType through the desk or +`bench --site console` (module `BWH Payments`, `issingle: 1`, System Manager permissions), then +`bench --site migrate`. Mirror the Razorpay field layout: + +- `enabled`, `test_mode`, plus any capture or notification switches (Check) +- credentials: public ids as Data, secrets as **Password** (required where the API can't work without them) +- `webhook_secret` (Password) and, if the provider publishes IPs, `webhook_ips` (Small Text) +- `currency` (Link), `success_url`, `failure_url`, `cancelled_url` (Data) + +Migrate regenerates the `# begin: auto-generated types` block in the controller. Don't write it by hand. + +## 3. Build the service client + +Follow AGENTS.md "Service clients" and the migrate skill steps 3–5 for `base.py`, `live.py` and `stub.py`. +For a new provider: + +- `base.py` methods map one to one to the provider calls you listed in step 1. Model only the fields the + controller reads. Every field on the refund model is optional, `id` and `amount` included (see the + migrate skill, step 3). +- `stub.py` must behave like the provider's real state machine for the paths you support: created, then + paid or authorised, then captured, then refunded. Include the refusal paths (`next_refund_status`, a + rejection control) and raise for unknown ids the way live does. + +## 4. Write the controller + +`class GatewaySettings(Document, PaymentGatewayBase)` with `get_gateway_name()` and `get_client()` +(stub iff `frappe.in_test`). Then implement the contract: + +- **`create_session(amount, currency, reference, customer)` → `CheckoutSession`.** + - Validate the currency with `validate_transaction_currency`, plus the provider's own allowlist if it + has one. + - Convert amounts only through `bwh_payments/currency.py`. + - Build return URLs with `urlsplit` and `parse_qsl`, append `reference_id=`, and run them + through `get_localised_url`. + - Drop customer fields the provider rejects when blank, rather than letting checkout fail. + - `session_id` must be the id webhooks and status reads use. +- **`get_payment_status(session_id)` → a Gateway Payment Request status.** + - Normalise once (`strip().casefold()`), then use a status map in which anything unrecognised is + `Pending`. + - A partial or authorised-but-uncaptured payment is never `Paid` unless the store's switch says so. If + the provider needs an explicit capture, do it here, for the authorised amount only (see Tabby). +- **`handle_webhook(payload, headers)` → `WebhookEvent | None`.** + - Verify first: HMAC over the raw bytes with `hmac.compare_digest`, guard non-ASCII signatures with + `isascii()`, and `frappe.throw` on a missing secret, header or event id. + - Return `None` for events you don't act on. + - Throw on a "paid" event you can't read, so the provider retries and it lands in the Error Log. + - Header names must match werkzeug's title-case (`X-Foo-Signature`), because the handler receives a + plain dict. +- **`refund_payment(session_id, amount, currency)` → `RefundResult`** with `amount` in major units. The + request reads only `refund_id`, and it reads it after the money has moved, so return what you have + rather than raising on a thin provider response. Use the provider's echoed amount when it sends one, + and the requested `amount` when it does not. + Refuse when there is nothing captured to refund, and throw on any status that isn't an accepted one. +- **`cancel_session(session_id)` → bool,** if the provider can cancel or expire a session. Return + `False` instead of raising when the provider refuses. + +Mark deliberate limits with `# ponytail:` comments that name the ceiling and the upgrade path (a missing +void endpoint, no timeout on `make_request`, and so on). + +## 5. Tests (stub-driven, in the doctype folder) + +`test__gateway_settings.py` with `StubXClient.reset()` in `setUp`. Cover at least: + +- amounts for a 2-, a 3- and a 0-decimal currency, as sent to the provider +- an unsupported currency refused before any call +- the session id being the one webhooks use, and the return URLs carrying `reference_id` +- the status map: each known status, an unknown one staying `Pending`, and authorised versus captured +- capture, if any: triggered once, for the authorised amount +- webhooks: valid, forged, missing header, wrong secret (for example the API key in place of the webhook + secret), an ignored event type, and an unreadable paid event +- refunds: accepted, refused, nothing captured, unknown session +- `release_if_unpaid` on an open session and on a paid one +- one spine test through `bwh_payments.bwh_payments.webhook.handle` (see `tests/test_razorpay_webhook.py`) + +If the class writes Integration Requests or Profiles, clean up in `tearDownClass` (see +`remove_razorpay_gateway`), because Frappe 17 rolls back only at class end and `create_request_log` +commits. + +## 6. Make it selectable + +- **Payment Gateway Profile:** a record named after the provider, with `gateway_settings` set to the new + Single. Its validate rejects a Single that doesn't implement the contract. +- **ERPNext refunds** need a Mode of Payment named exactly like the profile. +- **Commera:** add an entry to `PAYMENT_GATEWAYS` in `commera/api/admin/payments.py` (slug, label, blurb, + `settings_doctype`, docs URL). Enabling it there creates the Mode of Payment, and the webhook URL shown + to the merchant is `/api/method/bwh_payments.bwh_payments.webhook.handle?gateway=`. +- For an end-to-end guarantee in commera, copy `commera/tests/test_razorpay_checkout.py` and swap the + driver helpers for the new stub. + +## 7. Document and verify + +- Add a "Gateway-specific behavior" bullet to AGENTS.md: transport, webhook scheme, and anything + surprising. +- Run the full bwh_payments and consumer suites, `pre-commit run --files ...`, and a leak check (no + stray profiles or enabled settings on the site). +- One commit in bwh_payments (`feat: add gateway`), and a separate one for the commera wiring. diff --git a/.claude/skills/migrate-payment-gateway/SKILL.md b/.claude/skills/migrate-payment-gateway/SKILL.md new file mode 100644 index 0000000..72bacf7 --- /dev/null +++ b/.claude/skills/migrate-payment-gateway/SKILL.md @@ -0,0 +1,173 @@ +--- +name: migrate-payment-gateway +description: Move an existing bwh_payments gateway (or any Frappe integration controller that calls make_post_request / make_get_request / an SDK directly) onto the base/live/stub service-client layout in bwh_payments/services/GATEWAY/, with typed pydantic models and tests that drive a stub instead of patching transports. Use this whenever someone wants to refactor, restructure, "clean up", make testable, or "move to the services pattern" a Gateway Settings controller, delete a tests/fake_*.py, stop patching HTTP in tests, or add stubs for a payment, shipping or other third-party integration, even if they don't say "migrate". +--- + +# Migrate a gateway to base/live/stub service clients + +The target layout and its rules are in `AGENTS.md` under "Service clients (base / live / stub)". Read that +section first; it is the source of truth, and this skill is the procedure for getting there safely. The +four gateways already migrated (`bwh_payments/services/{razorpay,stripe,telr,tabby}/`) are worked +examples. Razorpay (raw HTTP) and Stripe (SDK) are the cleanest to copy from. + +The whole point of the migration is that **behaviour does not change**. So the procedure is built around +proving that: pin current behaviour, move code in thin slices, and re-run the same tests after each slice. + +## 0. Set up + +- Branch in **every** repo you will touch (bwh_payments and each consumer app such as commera) before the + first edit. +- Integration tests need the bench's Redis. If they crash with `Error 61 connecting to 127.0.0.1:`, + start it from the bench root: `redis-server config/redis_cache.conf --daemonize yes`, and the same for + `config/redis_queue.conf`. Or use `bench start`. +- Run the baseline and write the counts down: + `bench --site run-tests --app bwh_payments`, then the same for each consumer app. You will + compare against these numbers at the end. + +## 1. Pin behaviour before touching anything + +Look at what the gateway's existing tests and the consumer apps' tests actually exercise. If a consumer +(commera, buzz) has no end-to-end test through this gateway, write one first, against the old code: +checkout, then paid via webhook, then paid via the return page, then abandoned-cart release, then refund. +`commera/tests/test_razorpay_checkout.py` is the template. + +- Put every reference to the transport fake in a small block of "gateway driver" helpers (`shopper_pays`, + `created_links`, `refunds_sent`, `link_status`, `checkout_url`). The migration may change those + helper bodies and nothing else. That is the parity proof. +- Prove the test can fail. Temporarily break one decision in the controller (for example map `paid` to + `Pending`), run the test, see it go red, then restore the file with `git checkout -- `. + +## 2. Sort every line of the controller into transport or decision + +Read the controller top to bottom and classify: + +| Transport: moves to `live.py` | Decision: stays in the controller | +|---|---| +| base URL, auth headers, Basic/Bearer tokens | status maps (unknown → `Pending`, never `Paid`) | +| `make_post_request` / `make_get_request` / SDK calls | currency → minor units, decimal formatting | +| reading error bodies, non-JSON guards, `frappe.throw` on refusal | success/return URL building | +| `create_request_log` and summarising for logs | capture and refund rules, "authorised counts as paid" switches | +| request encoding (e.g. Telr's refund XML) and response parsing | webhook signature/token verification | + +Webhook verification always stays in the controller. It is local crypto, and if it lived in a client, the +stub that replaces the client under tests would accept forged events. + +## 3. Write `services//base.py` + +- One abstract method per gateway call the controller makes. Name it after the gateway's own operation + (`create_payment_link`, `check_order`, `capture_payment`). Pass request bodies in the gateway's own + shape (`payload: dict`) when the controller builds them, so recordings stay comparable to the docs. +- Models mirror the gateway's JSON, keeping only fields the controller reads. Subclass + `bwh_payments.services.GatewayModel`, which ignores unknown fields and coerces bare numbers into `str` + fields, so an id the gateway sometimes sends as `1234567` still parses. Use nested models where the JSON + nests, and `| None = None` wherever the gateway may omit a field or send null. Use + `str | float | None` for amounts that arrive both as `100` and as `"100.00"`. +- A model the controller reads after money has moved (a refund echo) has **no** required fields, not + even `id` or `amount`. If it fails to parse, the refund is never recorded and stays refundable, so a + retry refunds twice. The controller falls back to the requested amount when the echo has none. +- The docstring on the ABC says what a refusal raises (`frappe.ValidationError`, or the SDK's own error + type when the controller already catches that, as with Stripe). + +## 4. Write `live.py` + +Move the transport code out of the controller. Live clients are deliberately untested, so keep them to +transport only: call, log, translate errors, parse. If you catch yourself writing an `if` about payment +state here, it belongs in the controller. + +- Credentials go in the constructor. A secret that is optional on the settings (Telr's `remote_auth_key`) + goes as an argument to the one method that needs it. `get_password` raises when the field is empty, + and `get_client()` runs on every call. +- Turn every refusal into `frappe.ValidationError`, including a body that is not JSON: check + `isinstance(response, dict)` before reading it. Annotate the helper that throws + (`throw_request_error`) as `-> NoReturn`, so `response` after the `except` is never read unbound. +- Log against the settings Single (`reference_doctype = reference_docname = " Gateway Settings"`). + Log endpoints and ids only, never payloads: they carry keys and shopper contact details. +- SDK objects: Stripe SDK 15 objects are attribute-based, not dicts, so parse with + `Model.model_validate(obj, from_attributes=True)`. Call the SDK's current namespace (`StripeClient.v1`, + from 12.5); the deprecated top-level services warn on every call. +- Smoke-test the parsing offline: feed each model a payload shaped like the provider's documented + response, including null blocks and numeric ids. Live clients have no tests, so this is the only check + their parsing gets. + +## 5. Write `stub.py` + +An in-memory version of the gateway that behaves like it, not a mock that returns whatever a test wants. + +- State in `ClassVar`s, a `reset()` classmethod, ids from `frappe.generate_hash` (the `order_ref` column is + unique, and rows can outlive a test). +- Name the controls after real-world events: `pay()`, `authorise()`, `set_status()`, `add_()` for + objects that exist at the gateway without this app creating them, and `next_refund_status` for refusals. +- Recordings (`created_*`, `refunds`, `captures`) keep the gateway's request shape. +- Unknown ids and refused operations raise the same exception type the live client raises, with the same + message prefix. Controllers catch these (for example `cancel_session` returning `False`). +- Return copies (`model_copy(deep=True)`) so the controller cannot mutate the stub's store. +- Webhook signing and event builders for tests live in `stub.py` too, replacing the ones in + `tests/fake_*.py`. + +## 6. Rewire the controller, in a tracer slice first + +```python +def get_client(self) -> BaseXClient: + if frappe.in_test: + return StubXClient() + + return LiveXClient(self.key_id, self.get_password("key_secret")) +``` + +Rewire `create_session` alone first and run the gateway's tests. Failures that only come from tests still +reading the old fake are expected, and they show the stub path works end to end. Then rewire the rest: + +- `create_session` returns `CheckoutSession`, `refund_payment` returns `RefundResult`, and + `handle_webhook` returns `WebhookEvent | None` (`None` replaces the old `{}` for "ignore this delivery"). +- Helpers that took response dicts now take models. Replace `payment.get("status")` with + `payment.status`, and `.get("payments") or []` with `payment_link.payments or []`. +- Keep `frappe.in_test` as the only switch. No env var, no "no keys configured" fallback: on a real + site that would mark orders Paid with no money taken. + +## 7. Move the tests onto the stub + +- In `setUp`: `StubXClient.reset()`. Delete the `patch.object(..., "make_post_request", ...)` blocks. +- Replace fake reads with stub controls and recordings, one for one. Keep every assertion's meaning. If a + test patched a controller method that no longer exists (Telr's `get_order`), drive the same scenario + through stub controls instead. +- Result dict access becomes attribute access (`session["session_id"]` becomes `session.session_id`), and + `assertEqual(result, {})` becomes `assertIsNone(result)`. +- A gateway with no tests gets them now. The stub makes them cheap (see the Tabby tests). + +## 8. Fix consumers in the same change + +```bash +grep -rn "fake_\|Fake\|_gateway_settings\.\(stripe\|make_\)" --include='*.py' apps/ +``` + +Swap consumer tests one for one, with no assertion changes, and commit them separately in the consumer +repo. Say in the message which bwh_payments commit they need, because merge order matters. + +Also grep every app on the bench for controller methods you removed (`get_order`, `get_headers`, `post`, +`get_resource`) before deleting them. + +## 9. Delete the fake and verify + +- `git rm bwh_payments/tests/fake_.py`. +- Run both full suites. Counts must match the baseline plus any tests you added, with nothing skipped that + wasn't skipped before. +- Run the consumer parity test. Its diff should touch only imports and the driver helpers. +- Mutation-check one decision again, so the tests still bite with the stub in place. +- `pre-commit run --files `, twice if ruff-format rewrites something. +- Leak check: no Payment Gateway Profile, enabled settings or Integration Requests left behind on the site. + +## Test-harness gotchas (Frappe 17) + +- `IntegrationTestCase` rolls back only at **class** end, and `create_request_log` calls + `frappe.db.commit()`. A test class that logs (webhooks, refunds, live calls) either cleans up in + `tearDownClass` (`remove_*_gateway` helpers) or patches `frappe.db.commit` for the class and rolls back + itself. +- Importing another `TestCase` class by name into a test module makes unittest run its tests a second + time. Borrow its helpers inside your class body (`helper = OtherSuite.helper`, then `del OtherSuite`). +- `frappe.clear_cache()` with no arguments also empties the per-worker `site_cache`, including + `get_available_payment_modes`. + +## Commit shape + +One commit per gateway in bwh_payments (`refactor: move onto base/live/stub service clients`), +one matching commit in each consumer, and AGENTS.md updated when a rule changes. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..e7f5a16 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,156 @@ +# AGENTS.md + +This file provides guidance to AI coding agents (Claude Code, Codex and others) when working with code in this repository. + +## What this is + +`bwh_payments` is a Frappe app (Frappe 16+, Python 3.14) that takes payments through hosted checkout pages +on Stripe, Razorpay, Telr and Tabby, behind one gateway-agnostic API. ERPNext is optional. Developer docs: +https://docs.bwh.tech/bwh-payments/get-started/overview + +The repo sits inside a bench at `/Users/harsh/bwh/bwh_payments` (`apps/bwh_payments`); the local site is +`bwhpayments.localhost`. Run `bench` commands from the bench root. + +## Commands + +```bash +# Tests (CI uses a site called test_site; allow_tests must be set once per site) +bench --site bwhpayments.localhost set-config allow_tests true +bench --site bwhpayments.localhost run-tests --app bwh_payments + +# One module / one test +bench --site bwhpayments.localhost run-tests --module bwh_payments.tests.test_webhook +bench --site bwhpayments.localhost run-tests --module bwh_payments.tests.test_webhook --test test_a_correctly_signed_event_marks_the_request_paid +bench --site bwhpayments.localhost run-tests --doctype "Gateway Payment Request" + +# Lint / format (same hooks CI runs; ruff, tabs, line length 110) +pre-commit run --all-files + +# After editing a DocType JSON +bench --site bwhpayments.localhost migrate +``` + +CI (`.github/workflows/linter.yml`) also runs Frappe's semgrep rules and `pip-audit`. + +## Architecture + +### The gateway contract + +`bwh_payments/base_class.py` defines `PaymentGatewayBase` (ABC): `create_session`, `get_payment_status`, +`refund_payment`, `handle_webhook`, plus optional `cancel_session`. Results are pydantic models: +`CheckoutSession`, `RefundResult`, and `WebhookEvent | None`. Callers read them through +`.model_validate`, so a third-party gateway that still returns dicts keeps working. Each gateway is a +**Single DocType** ` Gateway Settings` whose controller inherits `(Document, PaymentGatewayBase)` and +holds the credentials (as `Password` fields, read with `get_password`). + +A **Payment Gateway Profile** (named record, e.g. "Stripe") links to one settings Single via +`gateway_settings`. Its validate rejects any Single that does not implement the contract. Everything else +reaches a gateway only through `frappe.get_single(profile.gateway_settings)`, so a third-party app can add +a gateway without touching this app. + +### Service clients (base / live / stub) + +Every external service sits behind a client in `bwh_payments/services//`: + +- `base.py`: an ABC of the calls the controller makes, plus models that mirror the service's own JSON. They + subclass `bwh_payments.services.GatewayModel`, which ignores unknown fields and reads bare numbers into + `str` fields. They keep only the fields we read, and are optional wherever the service sends null. +- `live.py`: the transport. It makes the HTTP or SDK call, logs it with `create_request_log` (endpoint and + ids only, never payloads), turns a refusal into `frappe.ValidationError`, and parses the response into the + model. It holds no business decisions. Live clients are not tested, so anything that branches on payment + state belongs in the controller. +- `stub.py`: an in-memory version of the service with the same methods. State lives in `ClassVar`s. + Tests drive outcomes through classmethod controls (`pay`, `authorise`, `set_status`, `add_*`, + `next_refund_status`) and call `reset()` in `setUp`. Recordings (`created_*`, `refunds`) keep the + service's own request shapes. Webhook signing and event builders for tests live here too. + +The settings controller picks the client in `get_client()`: the stub when `frappe.in_test`, otherwise the +live one. There is no other switch, so a real site can never run on a stub. The controller keeps every +decision: currency conversion, URL building, status maps, capture and refund rules, and webhook signature +checks. Signature checks are local crypto and must never move into a client a stub could replace. + +To add a service, copy `services/razorpay/` and wire `get_client()` the same way. + +### Public API for other apps + +`bwh_payments/api.py` is the supported import surface: `create_payment(ref_doctype, ref_docname, amount, +currency, gateway, **fields)` (`fields` limited to `company` and `customer_*`), `get_payment_gateways`, `resolve_payment_mode`, `to_minor_units`, +`from_minor_units`. It skips permission checks, so never whitelist it as it stands. When a request turns +`Paid`, `on_update` calls `ref_doc.run_method("on_payment_authorized", "Completed")` once. This is the +frappe/payments convention, so apps written for it settle orders unchanged. + +### Payment flow + +1. Caller inserts a **Gateway Payment Request** (`ref_doctype`/`ref_docname`, `gateway` = profile name, + `amount`, `currency_code`, optional customer fields). +2. `before_save` calls `create_session` once (when `order_ref` is empty) and stores `order_ref` (gateway + session id, unique), `order_url` (checkout URL for the shopper) and the return URLs. +3. Status moves from `Pending` by one of two paths, both under a row lock (`lock_refund_ledger`): + - **Webhook**: `bwh_payments.bwh_payments.webhook.handle` (guest POST, `?gateway=`, + rate-limited) → settings `handle_webhook` verifies and returns a `WebhookEvent` or `None` + → request found by `order_ref` → `apply_webhook_status` as Administrator (dedupes on + `last_webhook_event_id`, only writes from `Pending`). + - **Pull**: whitelisted `sync_status` asks the gateway. Return pages must call this; never trust the + browser redirect. +4. `refund(amount=None, payment_entry=None)` does full or partial refunds with an over-refund guard, keeps + `refund_amount`, a comma-joined `refund_id` list, and sets `Partially Refunded` / `Refunded`. +5. ERPNext bridge (`hooks.py` `doc_events`): submitting a `Payment Entry` with `payment_type = "Pay"`, + `reference_no` = `order_ref` and `mode_of_payment` = the gateway profile name refunds through the + gateway. Amended entries are traced back via `amended_from` so a refund is never sent twice. + +### Gateway-specific behavior worth knowing + +- **Stripe**: official SDK through a `stripe.StripeClient` per `LiveStripeClient` (never module-level + `stripe.api_key`). SDK objects are attribute-based, so the live client parses with `from_attributes=True`. + The `stripe` version range in `pyproject.toml` is constrained by `frappe/payments` in the same venv; read + the comment there before changing it. +- **Razorpay**: raw HTTP (Payment Links API) via `frappe.integrations.utils.make_post_request` / + `make_get_request`, no SDK. +- **Telr**: no signed webhook. `handle_webhook` returns `None` on purpose; status comes only from + `get_payment_status`. +- **Tabby**: unsigned webhooks, checked with a static header token plus a source-IP allowlist. + `get_payment_status` also *captures* an authorised payment; that capture is where money is taken. + +### Money and currency rules + +- Amounts crossing `PaymentGatewayBase` are always in **major units**. Convert with + `bwh_payments/currency.py` (`to_minor_units` / `from_minor_units`), which uses the pinned + `MINOR_UNIT_EXPONENTS` table (ISO 4217). Do not read precision from the Currency DocType's + `fraction_units`; it is wrong for currencies such as JPY. +- Refund arithmetic rounds to the currency's minor-unit exponent, not the field precision. +- `GatewayPaymentRequest.validate_amount_precision` refuses amounts the site cannot store exactly (for + example KWD on a site without "Use Number Format From Currency"). + +### Security conventions in `webhook.py` + +Every verification failure returns the same opaque 400 (`reject()`), and `message_log` is cleared so a +`frappe.throw` from a verifier does not leak. A verified delivery that `WebhookEvent` cannot validate is a +400 too, so the gateway retries it and it lands in the Error Log. Do not log webhook payloads; they can contain cardholder +data. Every call is recorded as an Integration Request via `create_request_log`. + +## Tests + +Tests are `IntegrationTestCase` and never touch the network. Controllers pick their stub client under +tests on their own, so tests do not patch transports: call `StubClient.reset()` in `setUp`, drive +the outcome with its controls, and assert on its recordings. The real controller code runs every time. + +Frappe 17's `IntegrationTestCase` rolls back only at class end, and `create_request_log` (webhook and refund +logging) commits. That is why the Stripe and Razorpay fixtures come with `remove_*_gateway` helpers that run +in `tearDownClass`. + +Reuse the fixtures `configure_stripe_gateway` and `make_payment_request` (in +`test_gateway_payment_request.py`) and `configure_razorpay_gateway` (in `test_razorpay_gateway_settings.py`). +Tests inside a doctype folder set `IGNORE_TEST_RECORD_DEPENDENCIES` to avoid pulling in ERPNext test +records. Do not set it in `bwh_payments/tests/`: Frappe raises `NotImplementedError` outside a doctype +folder, and loads no test records there anyway. Webhook tests +build a werkzeug `Request` into `frappe.local.request` and call `webhook.handle()` directly. + +## Conventions + +- `hooks.py` sets `require_type_annotated_api_methods = True`: every `@frappe.whitelist` method needs + type annotations. `export_python_type_annotations = True` regenerates the `# begin: auto-generated types` + block in controllers on migrate; do not edit that block by hand. +- `use_json_request_body = True`: non-GET calls to this app's endpoints send JSON bodies. +- Deliberate shortcuts are marked with `# ponytail:` comments that name the limit and the upgrade path. +- `get_available_payment_modes` uses a per-worker `site_cache` (1 h TTL), so other workers can briefly + keep offering a just-disabled gateway. diff --git a/bwh_payments/api.py b/bwh_payments/api.py new file mode 100644 index 0000000..b572c31 --- /dev/null +++ b/bwh_payments/api.py @@ -0,0 +1,47 @@ +"""The supported entry points for other apps. Server-side only: nothing here checks permissions, so never +whitelist these as they stand.""" + +import frappe +from frappe import _ + +from bwh_payments.bwh_payments.doctype.gateway_payment_request.gateway_payment_request import ( + GatewayPaymentRequest, +) +from bwh_payments.bwh_payments.utils import get_available_payment_modes, resolve_payment_mode +from bwh_payments.currency import from_minor_units, to_minor_units + +__all__ = [ + "create_payment", + "from_minor_units", + "get_payment_gateways", + "resolve_payment_mode", + "to_minor_units", +] + +get_payment_gateways = get_available_payment_modes + + +def create_payment( + ref_doctype: str, ref_docname: str, amount: float, currency: str, gateway: str, **fields +) -> GatewayPaymentRequest: + """Open a gateway checkout for a document and return its Gateway Payment Request. + + Send the shopper to the request's `order_url`. `fields` takes any other request field, such as + `company` or the `customer_*` details the gateway prefills. The referenced document's + `on_payment_authorized(status)` runs once the payment is Paid. + """ + # The insert skips permissions, so a caller forwarding user input must not reach status or the ledger. + if blocked := [field for field in fields if field != "company" and not field.startswith("customer_")]: + frappe.throw(_("create_payment cannot set {0}").format(", ".join(sorted(blocked)))) + + return frappe.get_doc( + { + "doctype": "Gateway Payment Request", + "ref_doctype": ref_doctype, + "ref_docname": ref_docname, + "amount": amount, + "currency_code": currency, + "gateway": gateway, + **fields, + } + ).insert(ignore_permissions=True) diff --git a/bwh_payments/base_class.py b/bwh_payments/base_class.py index 9217561..7e33c43 100644 --- a/bwh_payments/base_class.py +++ b/bwh_payments/base_class.py @@ -1,5 +1,36 @@ from abc import ABC, abstractmethod +from pydantic import BaseModel, ConfigDict + + +class GatewayResult(BaseModel): + # A gateway from another app may hand back numeric ids. Rejecting one after a refund has moved money + # would leave it unrecorded and refundable again. + model_config = ConfigDict(coerce_numbers_to_str=True) + + +class CheckoutSession(GatewayResult): + session_id: str + redirect_url: str + success_url: str | None = None + cancel_url: str | None = None + failure_url: str | None = None + + +class RefundResult(GatewayResult): + # Read after the money has moved, so nothing here may be required: a thin result must not strand it. + refund_id: str | None = None + status: str | None = None + # Major units, round-tripped from the gateway's own echo where it gives one. + amount: float | None = None + + +class WebhookEvent(GatewayResult): + session_id: str + # A Gateway Payment Request `status` value. + status: str + event_id: str | None = None + class PaymentGatewayBase(ABC): """Contract every ` Gateway Settings` Single must satisfy to back a Payment Gateway Profile. @@ -8,6 +39,9 @@ class PaymentGatewayBase(ABC): them is its own business: Stripe converts to ISO minor units with `bwh_payments.currency.to_minor_units`, while Telr bills in major units and only uses the currency's minor-unit exponent to decide how many decimals to send. Either way charge and refund go through the same conversion, so they always agree. + + Callers read every result through `.model_validate`, so a gateway from another app that still + returns plain dicts keeps working, and one that returns garbage fails loudly at the boundary. """ @abstractmethod @@ -17,20 +51,18 @@ def create_session( currency: str, reference: str | None = None, customer: dict | None = None, - ) -> dict: - """Return {"session_id", "redirect_url", "success_url", "cancel_url", "failure_url"}.""" + ) -> CheckoutSession: ... @abstractmethod def get_payment_status(self, session_id: str) -> str: """Return one of the Gateway Payment Request `status` values, read from the gateway.""" @abstractmethod - def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> dict: - """Return {"refund_id", "status", "amount"} with amount in major units.""" + def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> RefundResult: ... @abstractmethod - def handle_webhook(self, payload: bytes, headers: dict) -> dict: - """Verify the signature, then return {} to ignore or {"session_id", "status", "event_id"}.""" + def handle_webhook(self, payload: bytes, headers: dict) -> WebhookEvent | None: + """Verify the signature, then return None to ignore the delivery or the event it carries.""" def cancel_session(self, session_id: str) -> bool: return False diff --git a/bwh_payments/bwh_payments/doctype/gateway_payment_request/gateway_payment_request.py b/bwh_payments/bwh_payments/doctype/gateway_payment_request/gateway_payment_request.py index 3c34b18..06148d8 100644 --- a/bwh_payments/bwh_payments/doctype/gateway_payment_request/gateway_payment_request.py +++ b/bwh_payments/bwh_payments/doctype/gateway_payment_request/gateway_payment_request.py @@ -7,6 +7,7 @@ from frappe.model.document import Document from frappe.utils.data import cstr, flt +from bwh_payments.base_class import CheckoutSession, RefundResult from bwh_payments.currency import get_minor_unit_exponent REFUNDABLE_STATUSES = ("Paid", "Partially Refunded") @@ -76,6 +77,14 @@ def before_save(self): if not self.order_ref: self.create_session() + def on_update(self): + # Every path to Paid (webhook, poll, sweep) saves through here, so the referenced document hears it + # exactly once. The frappe/payments convention, so an app written for that settles orders unchanged. + if self.has_value_changed("status") and self.status == "Paid": + frappe.get_doc(self.ref_doctype, self.ref_docname).run_method( + "on_payment_authorized", "Completed" + ) + def get_gateway_settings(self): gateway_settings = frappe.get_cached_value( "Payment Gateway Profile", self.gateway, "gateway_settings" @@ -83,17 +92,19 @@ def get_gateway_settings(self): return frappe.get_single(gateway_settings) def create_session(self): - session = self.get_gateway_settings().create_session( - flt(self.amount), - self.currency_code, - reference=self.name, - customer=self.get_customer_details(), + session = CheckoutSession.model_validate( + self.get_gateway_settings().create_session( + flt(self.amount), + self.currency_code, + reference=self.name, + customer=self.get_customer_details(), + ) ) - self.order_ref = session.get("session_id") - self.order_url = session.get("redirect_url") - self.success_url = session.get("success_url") - self.cancel_url = session.get("cancel_url") - self.failure_url = session.get("failure_url") + self.order_ref = session.session_id + self.order_url = session.redirect_url + self.success_url = session.success_url + self.cancel_url = session.cancel_url + self.failure_url = session.failure_url def get_customer_details(self) -> dict: address = None @@ -229,7 +240,7 @@ def refund(self, amount: float | None = None, payment_entry: str | None = None): request_log.db_set("status", "Failed", update_modified=False) raise - self.append_refund_id((result or {}).get("refund_id")) + self.append_refund_id(RefundResult.model_validate(result or {}).refund_id) self.refund_amount = flt(flt(self.refund_amount, precision) + amount, precision) self.status = self.resolve_refund_status() if payment_entry: diff --git a/bwh_payments/bwh_payments/doctype/gateway_payment_request/test_gateway_payment_request.py b/bwh_payments/bwh_payments/doctype/gateway_payment_request/test_gateway_payment_request.py index 5640872..3123b6b 100644 --- a/bwh_payments/bwh_payments/doctype/gateway_payment_request/test_gateway_payment_request.py +++ b/bwh_payments/bwh_payments/doctype/gateway_payment_request/test_gateway_payment_request.py @@ -4,12 +4,14 @@ from unittest.mock import patch import frappe +from frappe.geo.doctype.currency.currency import Currency from frappe.tests import IntegrationTestCase from frappe.utils.data import flt -from bwh_payments.bwh_payments.doctype.stripe_gateway_settings import stripe_gateway_settings +from bwh_payments.api import create_payment from bwh_payments.currency import to_minor_units -from bwh_payments.tests.fake_stripe import FakeStripeClient +from bwh_payments.services.stripe.base import StripeRefund +from bwh_payments.services.stripe.stub import StubStripeClient GATEWAY = "Stripe Test Gateway" WEBHOOK_SECRET = "whsec_test_secret" @@ -91,12 +93,7 @@ def make_payment_request(amount: float, currency: str = "SAR"): class TestGatewayPaymentRequest(IntegrationTestCase): def setUp(self): - FakeStripeClient.reset() - self.stripe_client_patch = patch.object( - stripe_gateway_settings.stripe, "StripeClient", FakeStripeClient - ) - self.stripe_client_patch.start() - self.addCleanup(self.stripe_client_patch.stop) + StubStripeClient.reset() configure_stripe_gateway() @classmethod @@ -114,9 +111,7 @@ def use_currency_number_format(self): frappe.clear_cache() def mark_paid(self, payment_request): - FakeStripeClient.register_paid_session( - payment_request.order_ref, payment_request.currency_code.lower() - ) + StubStripeClient.pay(payment_request.order_ref) payment_request.db_set("status", "Paid", update_modified=False) payment_request.reload() @@ -127,7 +122,7 @@ def test_create_session_charges_iso_minor_units_for_a_three_decimal_currency(sel self.use_currency_number_format() payment_request = make_payment_request(12.345, "KWD") - charged = FakeStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] + charged = StubStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] self.assertEqual(charged, 12345) self.assertNotEqual(charged, int(12.345 * 100)) self.assertTrue(payment_request.order_ref) @@ -140,12 +135,12 @@ def test_a_three_decimal_amount_is_refused_when_the_site_stores_only_two(self): with self.assertRaises(frappe.ValidationError): make_payment_request(12.345, "KWD") - self.assertEqual(FakeStripeClient.created_sessions, []) + self.assertEqual(StubStripeClient.created_sessions, []) def test_create_session_charges_iso_minor_units_for_a_zero_decimal_currency(self): make_payment_request(1000, "JPY") - charged = FakeStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] + charged = StubStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] self.assertEqual(charged, 1000) self.assertNotEqual(charged, 1000 * 100) @@ -165,12 +160,12 @@ def test_success_url_keeps_the_stripe_placeholder_and_existing_query(self): def test_charge_and_refund_agree_on_the_minor_unit_conversion(self): self.use_currency_number_format() payment_request = make_payment_request(12.345, "KWD") - charged = FakeStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] + charged = StubStripeClient.created_sessions[-1]["line_items"][0]["price_data"]["unit_amount"] self.mark_paid(payment_request) payment_request.refund() - self.assertEqual(FakeStripeClient.created_refunds[-1]["amount"], charged) + self.assertEqual(StubStripeClient.created_refunds[-1]["amount"], charged) self.assertEqual(to_minor_units(payment_request.refund_amount, "KWD"), charged) self.assertEqual(payment_request.status, "Refunded") @@ -188,7 +183,7 @@ def test_refund_guard_reads_the_locked_ledger_not_the_in_memory_copy(self): with self.assertRaises(frappe.ValidationError): payment_request.refund(100) - self.assertEqual(FakeStripeClient.created_refunds, []) + self.assertEqual(StubStripeClient.created_refunds, []) def test_refund_id_is_appended_once_per_successful_partial(self): payment_request = make_payment_request(100, "SAR") @@ -207,12 +202,12 @@ def test_over_refund_is_rejected_and_never_reaches_the_gateway(self): payment_request = make_payment_request(100, "SAR") self.mark_paid(payment_request) payment_request.refund(60) - FakeStripeClient.created_refunds.clear() + StubStripeClient.created_refunds.clear() with self.assertRaises(frappe.ValidationError): payment_request.refund(41) - self.assertEqual(FakeStripeClient.created_refunds, []) + self.assertEqual(StubStripeClient.created_refunds, []) self.assertEqual(flt(payment_request.refund_amount), 60.0) def test_an_omitted_amount_refunds_the_whole_remaining_balance(self): @@ -233,7 +228,7 @@ def test_a_sub_minor_unit_amount_is_rejected_rather_than_refunding_everything(se with self.assertRaises(frappe.ValidationError): payment_request.refund(0.004) - self.assertEqual(FakeStripeClient.created_refunds, []) + self.assertEqual(StubStripeClient.created_refunds, []) payment_request.reload() self.assertEqual(flt(payment_request.refund_amount), 0.0) self.assertEqual(payment_request.status, "Paid") @@ -245,7 +240,7 @@ def test_an_explicit_zero_is_rejected_rather_than_refunding_everything(self): with self.assertRaises(frappe.ValidationError): payment_request.refund(0) - self.assertEqual(FakeStripeClient.created_refunds, []) + self.assertEqual(StubStripeClient.created_refunds, []) self.assertEqual(flt(payment_request.refund_amount), 0.0) def test_a_residue_under_a_whole_unit_stays_refundable(self): @@ -260,7 +255,7 @@ def test_a_residue_under_a_whole_unit_stays_refundable(self): self.assertEqual(flt(payment_request.refund_amount), 100.0) self.assertEqual(payment_request.status, "Refunded") - self.assertEqual(FakeStripeClient.created_refunds[-1]["amount"], to_minor_units(0.99, "SAR")) + self.assertEqual(StubStripeClient.created_refunds[-1]["amount"], to_minor_units(0.99, "SAR")) def test_a_whole_unit_left_is_still_only_partially_refunded(self): payment_request = make_payment_request(100, "SAR") @@ -276,12 +271,12 @@ def test_refund_rejected_while_the_payment_is_still_pending(self): with self.assertRaises(frappe.ValidationError): payment_request.refund(10) - self.assertEqual(FakeStripeClient.created_refunds, []) + self.assertEqual(StubStripeClient.created_refunds, []) def test_a_failed_gateway_refund_leaves_the_ledger_untouched(self): payment_request = make_payment_request(100, "SAR") self.mark_paid(payment_request) - FakeStripeClient.next_refund_status = "failed" + StubStripeClient.next_refund_status = "failed" with self.assertRaises(frappe.ValidationError): payment_request.refund(50) @@ -290,6 +285,39 @@ def test_a_failed_gateway_refund_leaves_the_ledger_untouched(self): self.assertEqual(flt(payment_request.refund_amount), 0.0) self.assertIsNone(payment_request.refund_id) + def test_a_gateway_that_returns_no_refund_details_still_books_the_refund(self): + """The result is read after the money has moved; a thin one must not strand the ledger.""" + payment_request = make_payment_request(100, "SAR") + self.mark_paid(payment_request) + + with patch.object(type(payment_request.get_gateway_settings()), "refund_payment", return_value=None): + payment_request.refund(40) + + self.assertEqual(flt(payment_request.refund_amount), 40.0) + self.assertEqual(payment_request.status, "Partially Refunded") + + def test_a_numeric_refund_id_from_another_apps_gateway_still_books_the_refund(self): + payment_request = make_payment_request(100, "SAR") + self.mark_paid(payment_request) + + with patch.object( + type(payment_request.get_gateway_settings()), "refund_payment", return_value={"refund_id": 12345} + ): + payment_request.refund(40) + + self.assertEqual(payment_request.refund_id, "12345") + self.assertEqual(flt(payment_request.refund_amount), 40.0) + + def test_a_refund_echo_without_id_or_amount_still_books_the_refund(self): + payment_request = make_payment_request(100, "SAR") + self.mark_paid(payment_request) + + with patch.object(StubStripeClient, "create_refund", return_value=StripeRefund(status="succeeded")): + payment_request.refund(40) + + self.assertEqual(flt(payment_request.refund_amount), 40.0) + self.assertEqual(payment_request.status, "Partially Refunded") + # --- polled status ---------------------------------------------------- def test_an_abandoned_session_is_released_and_expired_at_the_gateway(self): @@ -297,15 +325,15 @@ def test_an_abandoned_session_is_released_and_expired_at_the_gateway(self): self.assertTrue(payment_request.release_if_unpaid()) self.assertEqual(payment_request.status, "Cancelled") - self.assertEqual(FakeStripeClient.sessions[payment_request.order_ref]["status"], "expired") + self.assertEqual(StubStripeClient.sessions[payment_request.order_ref].status, "expired") def test_a_paid_session_keeps_its_payment_and_is_never_expired(self): payment_request = make_payment_request(100, "SAR") - FakeStripeClient.register_paid_session(payment_request.order_ref, "sar") + StubStripeClient.pay(payment_request.order_ref) self.assertFalse(payment_request.release_if_unpaid()) self.assertEqual(payment_request.status, "Paid") - self.assertEqual(FakeStripeClient.sessions[payment_request.order_ref]["status"], "complete") + self.assertEqual(StubStripeClient.sessions[payment_request.order_ref].status, "complete") def test_a_session_the_gateway_will_not_cancel_stays_pending(self): payment_request = make_payment_request(100, "SAR") @@ -368,6 +396,42 @@ def test_a_webhook_cannot_write_a_refund_status(self): self.assertFalse(payment_request.apply_webhook_status("Refunded", "evt_1")) self.assertEqual(payment_request.status, "Pending") + # --- reference document callback --------------------------------------- + + def test_the_referenced_document_hears_about_a_payment_once(self): + payment_request = make_payment_request(100, "SAR") + + with patch.object(Currency, "on_payment_authorized", create=True) as on_payment_authorized: + payment_request.apply_webhook_status("Paid", "evt_1") + payment_request.sync_status() + + on_payment_authorized.assert_called_once_with("Completed") + + def test_a_payment_that_did_not_settle_is_not_announced(self): + payment_request = make_payment_request(100, "SAR") + + with patch.object(Currency, "on_payment_authorized", create=True) as on_payment_authorized: + payment_request.apply_webhook_status("Cancelled", "evt_1") + + on_payment_authorized.assert_not_called() + + # --- public api ------------------------------------------------------- + + def test_create_payment_opens_a_checkout_for_the_document(self): + payment_request = create_payment( + "Currency", "SAR", 100, "SAR", GATEWAY, customer_email="shopper@example.com" + ) + + self.assertEqual(payment_request.status, "Pending") + self.assertTrue(payment_request.order_url) + self.assertEqual(payment_request.customer_email, "shopper@example.com") + + def test_create_payment_refuses_fields_beyond_company_and_customer_details(self): + with self.assertRaises(frappe.ValidationError): + create_payment("Currency", "SAR", 100, "SAR", GATEWAY, status="Paid") + + self.assertEqual(StubStripeClient.created_sessions, []) + # --- schema invariants ------------------------------------------------ def test_order_ref_is_unique(self): diff --git a/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/razorpay_gateway_settings.py b/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/razorpay_gateway_settings.py index 799a893..dfc6a0a 100644 --- a/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/razorpay_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/razorpay_gateway_settings.py @@ -7,19 +7,16 @@ import frappe from frappe import _ -from frappe.integrations.utils import create_request_log, make_get_request, make_post_request from frappe.model.document import Document from frappe.utils import validate_email_address from frappe.utils.data import flt -from bwh_payments.base_class import PaymentGatewayBase +from bwh_payments.base_class import CheckoutSession, PaymentGatewayBase, RefundResult, WebhookEvent from bwh_payments.bwh_payments.utils import get_localised_url from bwh_payments.currency import from_minor_units, to_minor_units, validate_transaction_currency - -# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Razorpay call holds a -# worker; revisit if Razorpay latency ever shows up in the request log. -RAZORPAY_BASE_URL = "https://api.razorpay.com/v1" -RAZORPAY_HEADERS = {"accept": "application/json", "Content-Type": "application/json"} +from bwh_payments.services.razorpay.base import BaseRazorpayClient, RazorpayPaymentLink +from bwh_payments.services.razorpay.live import LiveRazorpayClient +from bwh_payments.services.razorpay.stub import StubRazorpayClient # Anything unrecognised stays Pending: never terminal, and never Paid, so an unmapped Razorpay state can # neither release goods nor cancel a live order. Only values the webhook/poll path may write appear here. @@ -68,8 +65,11 @@ class RazorpayGatewaySettings(Document, PaymentGatewayBase): def get_gateway_name(self) -> str: return "Razorpay" - def get_auth(self) -> tuple[str, str]: - return (self.key_id, self.get_password("key_secret")) + def get_client(self) -> BaseRazorpayClient: + if frappe.in_test: + return StubRazorpayClient() + + return LiveRazorpayClient(self.key_id, self.get_password("key_secret")) def create_session( self, @@ -77,7 +77,7 @@ def create_session( currency: str, reference: str | None = None, customer: dict | None = None, - ) -> dict: + ) -> CheckoutSession: currency = currency or self.currency validate_transaction_currency(currency) @@ -101,16 +101,16 @@ def create_session( if customer_details := get_customer_details(customer or {}): payload["customer"] = customer_details - link = self.post("/payment_links", payload) + link = self.get_client().create_payment_link(payload) # A Payment Link carries a single callback, so Razorpay has no cancel or failure redirect of its # own; the storefront gets ours instead, which is what the shopper would have landed on anyway. - return { - "session_id": link["id"], - "redirect_url": link["short_url"], - "success_url": payload["callback_url"], - "cancel_url": get_localised_url(self.cancelled_url), - "failure_url": get_localised_url(self.failure_url), - } + return CheckoutSession( + session_id=link.id, + redirect_url=link.short_url, + success_url=payload["callback_url"], + cancel_url=get_localised_url(self.cancelled_url), + failure_url=get_localised_url(self.failure_url), + ) def build_success_url(self, reference: str | None) -> str: """Razorpay's callback names its parameter `razorpay_payment_link_reference_id`, but the @@ -123,14 +123,11 @@ def build_success_url(self, reference: str | None) -> str: query.append(("reference_id", reference or "")) return urlunsplit((parts.scheme, parts.netloc, parts.path, urlencode(query), parts.fragment)) - def get_payment_link(self, session_id: str) -> dict: - return self.get_resource(f"/payment_links/{session_id}") - def get_payment_status(self, session_id: str) -> str: - payment_link = self.get_payment_link(session_id) + payment_link = self.get_client().get_payment_link(session_id) # Normalised once: reading the raw field again below would let a " Created " past the map but not # past the eligibility check, so the two reads have to agree. - link_status = (payment_link.get("status") or "").strip().casefold() + link_status = (payment_link.status or "").strip().casefold() status = RAZORPAY_LINK_STATUS_MAP.get(link_status, "Pending") if status != "Pending": return status @@ -149,7 +146,7 @@ def get_payment_status(self, session_id: str) -> str: return status - def handle_webhook(self, payload: bytes, headers: dict) -> dict: + def handle_webhook(self, payload: bytes, headers: dict) -> WebhookEvent | None: webhook_secret = self.get_password("webhook_secret") if not webhook_secret: frappe.throw(_("Webhook secret is not configured in Razorpay Gateway Settings")) @@ -175,7 +172,7 @@ def handle_webhook(self, payload: bytes, headers: dict) -> dict: # parse_json passes bytes straight through untouched, so the body is decoded first. event = frappe.parse_json(payload.decode()) if event.get("event") != RAZORPAY_PAID_EVENT: - return {} + return None payment_link = ((event.get("payload") or {}).get("payment_link") or {}).get("entity") or {} session_id = payment_link.get("id") @@ -190,87 +187,30 @@ def handle_webhook(self, payload: bytes, headers: dict) -> dict: # The payment link id, not the payment id: `order_ref` holds the link id, and the webhook handler # matches on it byte-for-byte. - return {"session_id": session_id, "status": status, "event_id": event_id} + return WebhookEvent(session_id=session_id, status=status, event_id=event_id) def cancel_session(self, session_id: str) -> bool: try: - link = self.post(f"/payment_links/{session_id}/cancel", {}) + link = self.get_client().cancel_payment_link(session_id) except frappe.ValidationError: return False - return (link.get("status") or "").strip().casefold() == RAZORPAY_CANCELLED_STATUS + return (link.status or "").strip().casefold() == RAZORPAY_CANCELLED_STATUS - def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> dict: + def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> RefundResult: currency = currency or self.currency - payment_id = get_captured_payment_id(self.get_payment_link(session_id)) + client = self.get_client() + payment_id = get_captured_payment_id(client.get_payment_link(session_id)) - refund = self.post(f"/payments/{payment_id}/refund", {"amount": to_minor_units(amount, currency)}) - if refund.get("status") not in RAZORPAY_ACCEPTED_REFUND_STATUSES: - frappe.throw(_("Refund failed with status: {0}").format(refund.get("status"))) + refund = client.create_refund(payment_id, to_minor_units(amount, currency)) + if refund.status not in RAZORPAY_ACCEPTED_REFUND_STATUSES: + frappe.throw(_("Refund failed with status: {0}").format(refund.status)) # The gateway's own echo is round-tripped back to major units so a charge and its refund always # agree to the last minor unit. - return { - "refund_id": refund["id"], - "status": refund["status"], - "amount": flt(from_minor_units(refund["amount"], currency)), - } - - def post(self, endpoint: str, payload: dict) -> dict: - url = f"{RAZORPAY_BASE_URL}{endpoint}" - try: - response = make_post_request(url, auth=self.get_auth(), json=payload, headers=RAZORPAY_HEADERS) - except Exception as exception: - self.throw_request_error(url, exception) - # throw_request_error always raises today; the bare raise keeps `response` from ever being - # read unbound if that stops being true. - raise - - return self.read_response(url, response) - - def get_resource(self, endpoint: str) -> dict: - url = f"{RAZORPAY_BASE_URL}{endpoint}" - try: - response = make_get_request(url, auth=self.get_auth(), headers=RAZORPAY_HEADERS) - except Exception as exception: - self.throw_request_error(url, exception) - # throw_request_error always raises today; the bare raise keeps `response` from ever being - # read unbound if that stops being true. - raise - - return self.read_response(url, response) - - def throw_request_error(self, url: str, exception: Exception): - """Razorpay explains a non-2xx in a JSON error body, so the status code alone never names it.""" - description = read_razorpay_error(exception) - self.log_request(url, error=description or exception) - frappe.throw(_("Razorpay rejected the request: {0}").format(description or _("unknown error"))) - - def read_response(self, url: str, response) -> dict: - if not isinstance(response, dict): - self.log_request(url, error="Razorpay returned a non-JSON body") - frappe.throw(_("Razorpay returned an unreadable response")) - - if error := response.get("error"): - self.log_request(url, error=error.get("description") or error.get("code")) - frappe.throw(_("Razorpay rejected the request: {0}").format(error.get("description"))) - - self.log_request(url, output=summarise_razorpay_entity(response)) - return response - - def log_request(self, url: str, output=None, error=None): - # The request payload carries the API key and the shopper's contact details, so only the endpoint - # and the outcome are logged. - create_request_log( - {"endpoint": url}, - service_name="Razorpay", - is_remote_request=True, - reference_doctype=self.doctype, - reference_docname=self.name, - output=output, - error=error, - status="Failed" if error else "Completed", - ) + if refund.amount is not None: + amount = flt(from_minor_units(refund.amount, currency)) + return RefundResult(refund_id=refund.id, status=refund.status, amount=amount) def get_customer_details(customer: dict) -> dict: @@ -288,20 +228,20 @@ def get_customer_name(customer: dict) -> str: return " ".join(part for part in (name.get("forenames"), name.get("surname")) if part) -def has_authorised_payment(payment_link: dict) -> bool: +def has_authorised_payment(payment_link: RazorpayPaymentLink) -> bool: return any( - (payment.get("status") or "").strip().casefold() == RAZORPAY_AUTHORISED_STATUS - for payment in payment_link.get("payments") or [] + (payment.status or "").strip().casefold() == RAZORPAY_AUTHORISED_STATUS + for payment in payment_link.payments or [] ) -def get_captured_payment_id(payment_link: dict) -> str: +def get_captured_payment_id(payment_link: RazorpayPaymentLink) -> str: """A Payment Link is not a charge, so a refund has to go against the payment it collected.""" # ponytail: a request marked Paid through `treat_authorised_as_paid` holds an authorised-but-uncaptured # payment and cannot be refunded here; capture it first, or refund it from the Razorpay dashboard. - for payment in payment_link.get("payments") or []: - if (payment.get("status") or "").strip().casefold() == RAZORPAY_CAPTURED_STATUS: - return payment["payment_id"] + for payment in payment_link.payments or []: + if (payment.status or "").strip().casefold() == RAZORPAY_CAPTURED_STATUS: + return payment.payment_id frappe.throw( _( @@ -309,19 +249,3 @@ def get_captured_payment_id(payment_link: dict) -> str: " refunded from the Razorpay dashboard." ) ) - - -def summarise_razorpay_entity(response: dict) -> dict: - """Razorpay echoes the shopper's name, email and phone number, so only the ids are logged.""" - return {"id": response.get("id"), "status": response.get("status")} - - -def read_razorpay_error(exception: Exception) -> str | None: - response = getattr(exception, "response", None) - if response is None: - return None - try: - body = response.json() - except ValueError: - return None - return ((body or {}).get("error") or {}).get("description") diff --git a/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/test_razorpay_gateway_settings.py b/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/test_razorpay_gateway_settings.py index 307b45b..6b4b5cb 100644 --- a/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/test_razorpay_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/razorpay_gateway_settings/test_razorpay_gateway_settings.py @@ -6,12 +6,7 @@ import frappe from frappe.tests import IntegrationTestCase -from bwh_payments.bwh_payments.doctype.razorpay_gateway_settings import razorpay_gateway_settings -from bwh_payments.tests.fake_razorpay import ( - FakeRazorpay, - build_payment_link_paid_event, - sign_razorpay_payload, -) +from bwh_payments.services.razorpay.stub import StubRazorpayClient, build_payment_link_event, sign_payload RAZORPAY_GATEWAY = "Razorpay" RAZORPAY_WEBHOOK_SECRET = "rzp_whsec_test_secret" @@ -59,9 +54,8 @@ def configure_razorpay_gateway(): def remove_razorpay_gateway(): """Undo configure_razorpay_gateway. - This matters more here than it does for Stripe: the Razorpay controller calls `create_request_log` on - every single HTTP call, and that helper ends in an unconditional `frappe.db.commit()`. So every test - that touches the transport escapes the per-test rollback and pins its rows to the site. Without this + Webhook deliveries and refunds log through `create_request_log`, which ends in an unconditional + `frappe.db.commit()`, so those tests escape the rollback and pin their rows to the site. Without this the dev site is left with an enabled gateway backed by a fake `rzp_test_x` key, which the storefront then offers shoppers at checkout. """ @@ -119,15 +113,7 @@ def make_razorpay_payment_request(amount: float, currency: str = "INR"): class RazorpayTestCase(IntegrationTestCase): def setUp(self): - FakeRazorpay.reset() - # Raw HTTP, no SDK: the seam is the two request helpers imported into the controller's namespace. - for helper, replacement in ( - ("make_post_request", FakeRazorpay.post), - ("make_get_request", FakeRazorpay.get), - ): - transport_patch = patch.object(razorpay_gateway_settings, helper, replacement) - transport_patch.start() - self.addCleanup(transport_patch.stop) + StubRazorpayClient.reset() configure_razorpay_gateway() @classmethod @@ -154,26 +140,26 @@ def use_currency_number_format(self): def test_create_session_charges_iso_minor_units_for_a_two_decimal_currency(self): self.get_settings().create_session(1234.56, "INR", reference="GPR-0001") - self.assertEqual(FakeRazorpay.created_links[-1]["amount"], 123456) + self.assertEqual(StubRazorpayClient.created_links[-1]["amount"], 123456) def test_create_session_charges_iso_minor_units_for_a_three_decimal_currency(self): """KWD has three decimals. An int(amount * 100) would bill 1234 fils for 12.345 KWD.""" self.get_settings().create_session(12.345, "KWD", reference="GPR-0002") - charged = FakeRazorpay.created_links[-1]["amount"] + charged = StubRazorpayClient.created_links[-1]["amount"] self.assertEqual(charged, 12345) self.assertNotEqual(charged, int(12.345 * 100)) def test_create_session_returns_the_link_id_and_short_url(self): session = self.get_settings().create_session(100, "INR", reference="GPR-0003") - self.assertTrue(session["session_id"].startswith("plink_")) - self.assertEqual(session["redirect_url"], f"https://rzp.io/i/{session['session_id']}") - self.assertIn("reference_id=GPR-0003", session["success_url"]) + self.assertTrue(session.session_id.startswith("plink_")) + self.assertEqual(session.redirect_url, StubRazorpayClient.links[session.session_id].short_url) + self.assertIn("reference_id=GPR-0003", session.success_url) def create_session_for_customer(self, customer): self.get_settings().create_session(100, "INR", reference="GPR-0100", customer=customer) - return FakeRazorpay.created_links[-1].get("customer", {}) + return StubRazorpayClient.created_links[-1].get("customer", {}) def test_a_valid_email_reaches_razorpay(self): customer = self.create_session_for_customer({"email": "shopper@example.com", "phone": "9876543210"}) @@ -187,16 +173,16 @@ def test_an_address_razorpay_would_refuse_is_dropped_rather_than_failing_the_lin self.assertEqual(customer["contact"], "9876543210") def test_an_unpaid_link_is_cancelled(self): - link_id = FakeRazorpay.register_link(status="created") + link_id = StubRazorpayClient.add_link("created") self.assertTrue(self.get_settings().cancel_session(link_id)) - self.assertEqual(FakeRazorpay.links[link_id]["status"], "cancelled") + self.assertEqual(StubRazorpayClient.links[link_id].status, "cancelled") def test_a_paid_link_is_not_cancelled_and_does_not_throw(self): - link_id = FakeRazorpay.register_link(status="paid") + link_id = StubRazorpayClient.add_link("paid") self.assertFalse(self.get_settings().cancel_session(link_id)) - self.assertEqual(FakeRazorpay.links[link_id]["status"], "paid") + self.assertEqual(StubRazorpayClient.links[link_id].status, "paid") def test_an_unknown_link_is_not_cancelled(self): self.assertFalse(self.get_settings().cancel_session("plink_never_existed")) @@ -206,22 +192,21 @@ def test_an_abandoned_link_is_released_and_cancelled_at_razorpay(self): self.assertTrue(payment_request.release_if_unpaid()) self.assertEqual(payment_request.status, "Cancelled") - self.assertEqual(FakeRazorpay.links[payment_request.order_ref]["status"], "cancelled") + self.assertEqual(StubRazorpayClient.links[payment_request.order_ref].status, "cancelled") def test_a_link_paid_before_the_release_keeps_its_payment(self): payment_request = make_razorpay_payment_request(100) - FakeRazorpay.links[payment_request.order_ref]["status"] = "paid" + StubRazorpayClient.set_status(payment_request.order_ref, "paid") self.assertFalse(payment_request.release_if_unpaid()) self.assertEqual(payment_request.status, "Paid") - self.assertEqual(FakeRazorpay.links[payment_request.order_ref]["status"], "paid") + self.assertEqual(StubRazorpayClient.links[payment_request.order_ref].status, "paid") def test_a_link_paid_between_the_status_read_and_the_cancel_keeps_its_payment(self): payment_request = make_razorpay_payment_request(100) - link = FakeRazorpay.links[payment_request.order_ref] def pay_then_refuse_cancel(session_id): - link["status"] = "paid" + StubRazorpayClient.set_status(session_id, "paid") return False with patch.object( @@ -237,9 +222,9 @@ def pay_then_refuse_cancel(session_id): # --- status map ------------------------------------------------------- def get_payment_status(self, status, payment_status=None, treat_authorised_as_paid=0): - link_id = FakeRazorpay.register_link(status=status) + link_id = StubRazorpayClient.add_link(status) if payment_status: - FakeRazorpay.add_payment(link_id, payment_status) + StubRazorpayClient.add_payment(link_id, payment_status) settings = self.get_settings() settings.treat_authorised_as_paid = treat_authorised_as_paid return settings.get_payment_status(link_id) @@ -319,40 +304,40 @@ def test_build_success_url_throws_when_the_success_url_is_unset(self): # --- refund ----------------------------------------------------------- def test_refund_resolves_the_captured_payment_and_posts_minor_units(self): - link_id = FakeRazorpay.register_link(status="paid") - FakeRazorpay.add_payment(link_id, "failed") - payment_id = FakeRazorpay.add_payment(link_id, "captured") + link_id = StubRazorpayClient.add_link("paid") + StubRazorpayClient.add_payment(link_id, "failed") + payment_id = StubRazorpayClient.add_payment(link_id, "captured") refund = self.get_settings().refund_payment(link_id, 12.345, "KWD") - self.assertEqual(FakeRazorpay.created_refunds[-1]["payment_id"], payment_id) - self.assertEqual(FakeRazorpay.created_refunds[-1]["amount"], 12345) + self.assertEqual(StubRazorpayClient.created_refunds[-1]["payment_id"], payment_id) + self.assertEqual(StubRazorpayClient.created_refunds[-1]["amount"], 12345) # The gateway's own echo round-tripped back to major units, so charge and refund agree. - self.assertEqual(refund["amount"], 12.345) - self.assertEqual(refund["status"], "processed") - self.assertTrue(refund["refund_id"].startswith("rfnd_")) + self.assertEqual(refund.amount, 12.345) + self.assertEqual(refund.status, "processed") + self.assertTrue(refund.refund_id.startswith("rfnd_")) def test_a_pending_refund_is_accepted(self): - link_id = FakeRazorpay.register_link(status="paid") - FakeRazorpay.add_payment(link_id, "captured") - FakeRazorpay.next_refund_status = "pending" + link_id = StubRazorpayClient.add_link("paid") + StubRazorpayClient.add_payment(link_id, "captured") + StubRazorpayClient.next_refund_status = "pending" - self.assertEqual(self.get_settings().refund_payment(link_id, 100, "INR")["status"], "pending") + self.assertEqual(self.get_settings().refund_payment(link_id, 100, "INR").status, "pending") def test_a_refund_throws_when_the_link_has_no_captured_payment(self): - link_id = FakeRazorpay.register_link(status="created") - FakeRazorpay.add_payment(link_id, "authorized") + link_id = StubRazorpayClient.add_link("created") + StubRazorpayClient.add_payment(link_id, "authorized") with self.assertRaises(frappe.ValidationError): self.get_settings().refund_payment(link_id, 100, "INR") - self.assertEqual(FakeRazorpay.created_refunds, []) + self.assertEqual(StubRazorpayClient.created_refunds, []) def test_a_refund_throws_when_the_gateway_reports_an_unaccepted_status(self): """A `failed` refund must not be recorded as money returned to the shopper.""" - link_id = FakeRazorpay.register_link(status="paid") - FakeRazorpay.add_payment(link_id, "captured") - FakeRazorpay.next_refund_status = "failed" + link_id = StubRazorpayClient.add_link("paid") + StubRazorpayClient.add_payment(link_id, "captured") + StubRazorpayClient.next_refund_status = "failed" with self.assertRaises(frappe.ValidationError): self.get_settings().refund_payment(link_id, 100, "INR") @@ -361,7 +346,7 @@ def test_a_refund_on_an_unknown_link_throws_rather_than_refunding_nothing_quietl with self.assertRaises(frappe.ValidationError): self.get_settings().refund_payment("plink_does_not_exist", 100, "INR") - self.assertEqual(FakeRazorpay.created_refunds, []) + self.assertEqual(StubRazorpayClient.created_refunds, []) class TestRazorpayWebhookVerification(RazorpayTestCase): @@ -376,59 +361,59 @@ def handle(self, payload: bytes, signature: str | None = None, event_id: str | N return self.get_settings().handle_webhook(payload, headers) def sign(self, payload: bytes, secret: str = RAZORPAY_WEBHOOK_SECRET) -> str: - return sign_razorpay_payload(payload, secret) + return sign_payload(payload, secret) def test_a_correctly_signed_paid_event_is_mapped_to_the_link_id_and_status(self): - payload = build_payment_link_paid_event("plink_signed_ok") + payload = build_payment_link_event("plink_signed_ok") result = self.handle(payload, self.sign(payload)) # The payment link id, not the payment id: `order_ref` holds the link id. - self.assertEqual(result["session_id"], "plink_signed_ok") - self.assertEqual(result["status"], "Paid") - self.assertEqual(result["event_id"], "evt_rzp_1") + self.assertEqual(result.session_id, "plink_signed_ok") + self.assertEqual(result.status, "Paid") + self.assertEqual(result.event_id, "evt_rzp_1") def test_a_forged_signature_is_rejected(self): - payload = build_payment_link_paid_event("plink_forged") + payload = build_payment_link_event("plink_forged") with self.assertRaises(frappe.ValidationError): self.handle(payload, self.sign(payload, "attacker_guess")) def test_a_signature_over_a_different_body_is_rejected(self): """Signing a re-serialised body is the classic way to accept a tampered payload.""" - signature = self.sign(build_payment_link_paid_event("plink_other")) + signature = self.sign(build_payment_link_event("plink_other")) with self.assertRaises(frappe.ValidationError): - self.handle(build_payment_link_paid_event("plink_swapped"), signature) + self.handle(build_payment_link_event("plink_swapped"), signature) def test_a_missing_signature_header_is_rejected(self): - payload = build_payment_link_paid_event("plink_unsigned") + payload = build_payment_link_event("plink_unsigned") with self.assertRaises(frappe.ValidationError): self.handle(payload, None) def test_the_key_secret_is_not_accepted_in_place_of_the_webhook_secret(self): """Razorpay signs with the webhook secret; reaching for `key_secret` fails every delivery.""" - payload = build_payment_link_paid_event("plink_wrong_secret") + payload = build_payment_link_event("plink_wrong_secret") with self.assertRaises(frappe.ValidationError): self.handle(payload, self.sign(payload, "rzp_test_secret")) def test_an_event_of_another_type_is_ignored(self): - payload = build_payment_link_paid_event("plink_captured", event="payment.captured") + payload = build_payment_link_event("plink_captured", event="payment.captured") - self.assertEqual(self.handle(payload, self.sign(payload)), {}) + self.assertIsNone(self.handle(payload, self.sign(payload))) def test_the_event_id_is_read_from_the_razorpay_header(self): - payload = build_payment_link_paid_event("plink_event_id") + payload = build_payment_link_event("plink_event_id") result = self.handle(payload, self.sign(payload), event_id="evt_from_header") - self.assertEqual(result["event_id"], "evt_from_header") + self.assertEqual(result.event_id, "evt_from_header") def test_an_unmapped_status_on_a_paid_event_throws_so_razorpay_retries(self): """Swallowing it would 200 a paid order into Pending with nothing in the Error Log.""" - payload = build_payment_link_paid_event("plink_odd_status", status="who_knows") + payload = build_payment_link_event("plink_odd_status", status="who_knows") with self.assertRaises(frappe.ValidationError): self.handle(payload, self.sign(payload)) @@ -436,7 +421,7 @@ def test_an_unmapped_status_on_a_paid_event_throws_so_razorpay_retries(self): def test_a_delivery_is_rejected_when_no_webhook_secret_is_configured(self): frappe.db.set_single_value("Razorpay Gateway Settings", "webhook_secret", "") frappe.clear_cache(doctype="Razorpay Gateway Settings") - payload = build_payment_link_paid_event("plink_no_secret") + payload = build_payment_link_event("plink_no_secret") with self.assertRaises(frappe.ValidationError): self.handle(payload, "anything") diff --git a/bwh_payments/bwh_payments/doctype/stripe_gateway_settings/stripe_gateway_settings.py b/bwh_payments/bwh_payments/doctype/stripe_gateway_settings/stripe_gateway_settings.py index 024db2a..3741841 100644 --- a/bwh_payments/bwh_payments/doctype/stripe_gateway_settings/stripe_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/stripe_gateway_settings/stripe_gateway_settings.py @@ -9,8 +9,11 @@ from frappe.model.document import Document from frappe.utils.data import flt -from bwh_payments.base_class import PaymentGatewayBase +from bwh_payments.base_class import CheckoutSession, PaymentGatewayBase, RefundResult, WebhookEvent from bwh_payments.currency import from_minor_units, to_minor_units, validate_transaction_currency +from bwh_payments.services.stripe.base import BaseStripeClient +from bwh_payments.services.stripe.live import LiveStripeClient +from bwh_payments.services.stripe.stub import StubStripeClient # Stripe substitutes this itself on redirect, so it has to survive URL encoding intact. STRIPE_SESSION_ID_PLACEHOLDER = "{CHECKOUT_SESSION_ID}" @@ -37,10 +40,11 @@ class StripeGatewaySettings(Document, PaymentGatewayBase): def get_gateway_name(self) -> str: return "Stripe" - def get_client(self) -> stripe.StripeClient: - # A client per call keeps two configured accounts from clobbering each other through the module - # level `stripe.api_key` that the SDK otherwise reads. - return stripe.StripeClient(self.get_password("private_key")) + def get_client(self) -> BaseStripeClient: + if frappe.in_test: + return StubStripeClient() + + return LiveStripeClient(self.get_password("private_key")) def create_session( self, @@ -48,9 +52,9 @@ def create_session( currency: str, reference: str | None = None, customer: dict | None = None, - ) -> dict: + ) -> CheckoutSession: validate_transaction_currency(currency) - session = self.get_client().checkout.sessions.create( + session = self.get_client().create_checkout_session( { "mode": "payment", "client_reference_id": reference, @@ -69,13 +73,13 @@ def create_session( "cancel_url": self.failure_url, } ) - return { - "session_id": session.id, - "redirect_url": session.url, - "success_url": session.success_url, - "cancel_url": session.cancel_url, - "failure_url": session.cancel_url, - } + return CheckoutSession( + session_id=session.id, + redirect_url=session.url, + success_url=session.success_url, + cancel_url=session.cancel_url, + failure_url=session.cancel_url, + ) def build_success_url(self) -> str: # String-concatenating "?session_id=..." breaks any success URL that already carries a query. @@ -89,7 +93,7 @@ def build_success_url(self) -> str: return urlunsplit((parts.scheme, parts.netloc, parts.path, encoded_query, parts.fragment)) def get_payment_status(self, session_id: str) -> str: - session = self.get_client().checkout.sessions.retrieve(session_id) + session = self.get_client().retrieve_checkout_session(session_id) if session.payment_status == "paid": return "Paid" if session.status == "expired": @@ -98,13 +102,13 @@ def get_payment_status(self, session_id: str) -> str: def cancel_session(self, session_id: str) -> bool: try: - session = self.get_client().checkout.sessions.expire(session_id) + session = self.get_client().expire_checkout_session(session_id) except stripe.StripeError: return False return session.status == "expired" - def handle_webhook(self, payload: bytes, headers: dict) -> dict: + def handle_webhook(self, payload: bytes, headers: dict) -> WebhookEvent | None: webhook_secret = self.get_password("webhook_secret") if not webhook_secret: frappe.throw(_("Webhook secret is not configured in Stripe Gateway Settings")) @@ -116,24 +120,24 @@ def handle_webhook(self, payload: bytes, headers: dict) -> dict: event = stripe.Webhook.construct_event(payload, signature, webhook_secret) if event["type"] != "checkout.session.completed": - return {} + return None session = event["data"]["object"] - return { - "session_id": session["id"], - "status": "Paid" if session["payment_status"] == "paid" else "Pending", - "event_id": event["id"], - } + return WebhookEvent( + session_id=session["id"], + status="Paid" if session["payment_status"] == "paid" else "Pending", + event_id=event["id"], + ) - def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> dict: + def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> RefundResult: client = self.get_client() - session = client.checkout.sessions.retrieve(session_id) + session = client.retrieve_checkout_session(session_id) if not session.payment_intent: frappe.throw(_("No payment intent found for this session; the payment did not complete.")) currency = currency or session.currency - refund = client.refunds.create( + refund = client.create_refund( { "payment_intent": session.payment_intent, "amount": to_minor_units(amount, currency), @@ -143,8 +147,6 @@ def refund_payment(self, session_id: str, amount: float, currency: str | None = if refund.status not in ("succeeded", "pending"): frappe.throw(_("Refund failed with status: {0}").format(refund.status)) - return { - "refund_id": refund.id, - "status": refund.status, - "amount": flt(from_minor_units(refund.amount, currency)), - } + if refund.amount is not None: + amount = flt(from_minor_units(refund.amount, currency)) + return RefundResult(refund_id=refund.id, status=refund.status, amount=amount) diff --git a/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/tabby_gateway_settings.py b/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/tabby_gateway_settings.py index 0558c1e..d918525 100644 --- a/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/tabby_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/tabby_gateway_settings.py @@ -7,17 +7,15 @@ import frappe from frappe import _ -from frappe.integrations.utils import create_request_log, make_get_request, make_post_request from frappe.model.document import Document from frappe.utils.data import flt -from bwh_payments.base_class import PaymentGatewayBase +from bwh_payments.base_class import CheckoutSession, PaymentGatewayBase, RefundResult, WebhookEvent from bwh_payments.bwh_payments.utils import get_localised_url from bwh_payments.currency import get_minor_unit_exponent, validate_transaction_currency - -# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Tabby call holds a worker; -# revisit if Tabby latency ever shows up in the request log. -TABBY_BASE_URL = "https://api.tabby.ai" +from bwh_payments.services.tabby.base import BaseTabbyClient, TabbyCheckout, TabbyConfiguration, TabbyPayment +from bwh_payments.services.tabby.live import LiveTabbyClient +from bwh_payments.services.tabby.stub import StubTabbyClient # Tabby only underwrites in these markets, and a merchant code is tied to one of them. Sending anything # else is a 400 the shopper sees as a broken checkout, so it is refused before the call is made. @@ -74,12 +72,11 @@ class TabbyGatewaySettings(Document, PaymentGatewayBase): def get_gateway_name(self) -> str: return "Tabby" - def get_headers(self) -> dict: - return { - "Authorization": f"Bearer {self.get_password('key_secret')}", - "Content-Type": "application/json", - "X-Merchant-Code": self.merchant_code, - } + def get_client(self) -> BaseTabbyClient: + if frappe.in_test: + return StubTabbyClient() + + return LiveTabbyClient(self.get_password("key_secret"), self.merchant_code) def create_session( self, @@ -87,7 +84,7 @@ def create_session( currency: str, reference: str | None = None, customer: dict | None = None, - ) -> dict: + ) -> CheckoutSession: currency = currency or self.currency validate_transaction_currency(currency) validate_tabby_currency(currency) @@ -112,24 +109,24 @@ def create_session( "merchant_urls": merchant_urls, } - checkout = self.post("/api/v2/checkout", payload) + checkout = self.get_client().create_checkout(payload) redirect_url = get_installments_url(checkout) # The *payment* id, not the top-level checkout id: `GET /payments/{id}`, the capture and refund # endpoints and the webhook body all key on the payment, and `webhook.handle` matches this value # against `order_ref` byte-for-byte. The checkout id is the obvious wrong choice and is unusable # everywhere else. - session_id = ((checkout.get("payment") or {}).get("id")) or "" + session_id = checkout.payment.id if checkout.payment else None if not session_id: frappe.throw(_("Tabby returned a checkout without a payment id")) - return { - "session_id": session_id, - "redirect_url": redirect_url, - "success_url": merchant_urls["success"], - "cancel_url": merchant_urls["cancel"], - "failure_url": merchant_urls["failure"], - } + return CheckoutSession( + session_id=session_id, + redirect_url=redirect_url, + success_url=merchant_urls["success"], + cancel_url=merchant_urls["cancel"], + failure_url=merchant_urls["failure"], + ) def build_return_url(self, url: str, label: str, reference: str | None) -> str: """Tabby only appends `?payment_id=...` on return, so the shopper carries our request name back.""" @@ -141,20 +138,20 @@ def build_return_url(self, url: str, label: str, reference: str | None) -> str: query.append(("reference_id", reference or "")) return urlunsplit((parts.scheme, parts.netloc, parts.path, urlencode(query), parts.fragment)) - def get_payment(self, session_id: str) -> dict: - return self.get_resource(f"/api/v2/payments/{session_id}") - def get_payment_status(self, session_id: str) -> str: - payment = self.get_payment(session_id) - status = read_status(payment) + client = self.get_client() + payment = client.get_payment(session_id) + status = read_status(payment.status) if status == TABBY_AUTHORISED_STATUS: - payment = self.capture_payment(session_id, payment) - status = read_status(payment) + payment = self.capture_payment(client, session_id, payment) + status = read_status(payment.status) return TABBY_STATUS_MAP.get(status, "Pending") - def capture_payment(self, session_id: str, payment: dict) -> dict: + def capture_payment( + self, client: BaseTabbyClient, session_id: str, payment: TabbyPayment + ) -> TabbyPayment: """Take the money Tabby has only authorised, and return the payment as it stands afterwards. This is where the shopper is actually charged: an authorised Tabby payment is approved credit and @@ -164,12 +161,11 @@ def capture_payment(self, session_id: str, payment: dict) -> dict: """ # The authorisation's own amount, never an argument: a capture can then never exceed what Tabby # approved, whatever the caller believes the order is worth. - currency = payment.get("currency") or self.currency - amount = format_tabby_amount(payment.get("amount"), currency) - self.post(f"/api/v2/payments/{session_id}/captures", {"amount": amount}) - return self.get_payment(session_id) + currency = payment.currency or self.currency + client.capture_payment(session_id, format_tabby_amount(payment.amount, currency)) + return client.get_payment(session_id) - def handle_webhook(self, payload: bytes, headers: dict) -> dict: + def handle_webhook(self, payload: bytes, headers: dict) -> WebhookEvent | None: self.validate_webhook_source_ip() webhook_secret = self.get_password("webhook_secret") @@ -193,9 +189,9 @@ def handle_webhook(self, payload: bytes, headers: dict) -> dict: try: payment = frappe.parse_json(payload.decode()) except ValueError: - return {} + return None if not isinstance(payment, dict): - return {} + return None session_id = payment.get("id") if not session_id: @@ -204,11 +200,11 @@ def handle_webhook(self, payload: bytes, headers: dict) -> dict: # ponytail: an `authorized` delivery is left Pending because capturing moves money and this path # runs as Guest, so a shopper who authorises and never returns to the site is not charged until a # status sync runs; add a scheduled sweep over Pending requests if abandoned returns show up. - return { - "session_id": session_id, - "status": TABBY_STATUS_MAP.get(read_status(payment), "Pending"), - "event_id": get_webhook_event_id(payment), - } + return WebhookEvent( + session_id=session_id, + status=TABBY_STATUS_MAP.get(read_status(payment.get("status")), "Pending"), + event_id=get_webhook_event_id(payment), + ) def validate_webhook_source_ip(self): """Defence in depth: the shared token is the only real check, so narrow who may even present it.""" @@ -218,10 +214,10 @@ def validate_webhook_source_ip(self): if frappe.local.request_ip not in allowed_ips: frappe.throw(_("Tabby webhook came from an address that is not allowed")) - def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> dict: + def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> RefundResult: currency = currency or self.currency - payment = self.get_payment(session_id) - if read_status(payment) != TABBY_CAPTURED_STATUS: + client = self.get_client() + if read_status(client.get_payment(session_id).status) != TABBY_CAPTURED_STATUS: # ponytail: only a captured payment is refundable — an authorised one has to be voided, and # Tabby's void endpoint is not wired up here; add it if orders start being cancelled between # authorisation and capture. @@ -234,71 +230,12 @@ def refund_payment(self, session_id: str, amount: float, currency: str | None = # Tabby answers a refund with the updated payment, not with a refund object, so the refund it just # created is the last entry on the payment's ledger. - updated_payment = self.post( - f"/api/v2/payments/{session_id}/refunds", {"amount": format_tabby_amount(amount, currency)} - ) - refunds = updated_payment.get("refunds") or [] + refunds = client.refund_payment(session_id, format_tabby_amount(amount, currency)).refunds or [] if not refunds: frappe.throw(_("Tabby accepted the refund but returned no refund record")) # Tabby reports no per-refund status; a refund it accepted and echoed back has succeeded. - return {"refund_id": refunds[-1].get("id"), "status": "succeeded", "amount": flt(amount)} - - def post(self, endpoint: str, payload: dict) -> dict: - url = f"{TABBY_BASE_URL}{endpoint}" - try: - response = make_post_request(url, json=payload, headers=self.get_headers()) - except Exception as exception: - self.throw_request_error(url, exception) - # throw_request_error always raises today; the bare raise keeps `response` from ever being - # read unbound if that stops being true. - raise - - return self.read_response(url, response) - - def get_resource(self, endpoint: str) -> dict: - url = f"{TABBY_BASE_URL}{endpoint}" - try: - response = make_get_request(url, headers=self.get_headers()) - except Exception as exception: - self.throw_request_error(url, exception) - # throw_request_error always raises today; the bare raise keeps `response` from ever being - # read unbound if that stops being true. - raise - - return self.read_response(url, response) - - def throw_request_error(self, url: str, exception: Exception): - """Tabby explains a non-2xx in a JSON error body, so the status code alone never names it.""" - description = read_tabby_error(exception) - self.log_request(url, error=description or exception) - frappe.throw(_("Tabby rejected the request: {0}").format(description or _("unknown error"))) - - def read_response(self, url: str, response) -> dict: - if not isinstance(response, dict): - self.log_request(url, error="Tabby returned a non-JSON body") - frappe.throw(_("Tabby returned an unreadable response")) - - if error := response.get("errorType"): - self.log_request(url, error=response.get("error") or error) - frappe.throw(_("Tabby rejected the request: {0}").format(response.get("error") or error)) - - self.log_request(url, output=summarise_tabby_payment(response)) - return response - - def log_request(self, url: str, output=None, error=None): - # The headers carry the Bearer secret and the payload carries the shopper's name, phone and - # address, so only the endpoint and the outcome are logged. - create_request_log( - {"endpoint": url}, - service_name="Tabby", - is_remote_request=True, - reference_doctype=self.doctype, - reference_docname=self.name, - output=output, - error=error, - status="Failed" if error else "Completed", - ) + return RefundResult(refund_id=refunds[-1].id, status="succeeded", amount=flt(amount)) def validate_tabby_currency(currency: str): @@ -320,9 +257,9 @@ def format_tabby_amount(amount, currency: str) -> str: return f"{flt(amount, exponent):.{exponent}f}" -def read_status(payment: dict) -> str: +def read_status(status: str | None) -> str: """Tabby's status is lowercase, but normalising once keeps a stray " Closed " from slipping past.""" - return ((payment or {}).get("status") or "").strip().casefold() + return (status or "").strip().casefold() def get_buyer_details(customer: dict) -> dict: @@ -348,45 +285,28 @@ def get_shipping_address(customer: dict) -> dict: return {key: value for key, value in details.items() if value} -def get_installments_url(checkout: dict) -> str: +def get_installments_url(checkout: TabbyCheckout) -> str: """Return the hosted checkout URL, or explain the refusal in words a shopper can act on.""" - configuration = checkout.get("configuration") or {} + configuration = checkout.configuration or TabbyConfiguration() # `available_products` holds a *list* of installment plans, while `products` holds an object carrying # the rejection reason. Indexing the list unguarded shows a declined shopper a raw traceback. - installments = ((configuration.get("available_products") or {}).get("installments")) or [] - if read_status(checkout) == TABBY_REJECTED_STATUS or not installments: + available_products = configuration.available_products + installments = (available_products.installments if available_products else None) or [] + if read_status(checkout.status) == TABBY_REJECTED_STATUS or not installments: throw_rejection(configuration) - web_url = (installments[0] or {}).get("web_url") + web_url = installments[0].web_url if not web_url: throw_rejection(configuration) return web_url -def throw_rejection(configuration: dict): - products = configuration.get("products") or {} - reason = ((products.get("installments") or {}).get("rejection_reason") or "").strip().casefold() +def throw_rejection(configuration: TabbyConfiguration): + product_status = configuration.products.installments if configuration.products else None + reason = ((product_status.rejection_reason if product_status else None) or "").strip().casefold() frappe.throw(_(TABBY_REJECTION_MESSAGES.get(reason, DEFAULT_REJECTION_MESSAGE)), title=_("Tabby")) -def summarise_tabby_payment(response: dict) -> dict: - """Tabby echoes the shopper's name, phone and address, so only the ids are logged.""" - # A checkout nests the payment; every other endpoint answers with the payment itself. - payment = response.get("payment") or response - return {"id": payment.get("id"), "status": response.get("status")} - - -def read_tabby_error(exception: Exception) -> str | None: - response = getattr(exception, "response", None) - if response is None: - return None - try: - body = response.json() - except ValueError: - return None - return (body or {}).get("error") or (body or {}).get("errorType") - - def get_webhook_event_id(payment: dict) -> str: """Tabby sends no event id, so replay protection needs a stable one derived from the delivery itself. diff --git a/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/test_tabby_gateway_settings.py b/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/test_tabby_gateway_settings.py new file mode 100644 index 0000000..6b9bc22 --- /dev/null +++ b/bwh_payments/bwh_payments/doctype/tabby_gateway_settings/test_tabby_gateway_settings.py @@ -0,0 +1,123 @@ +# Copyright (c) 2026, Build With Hussain and contributors +# See license.txt + +import json + +import frappe +from frappe.tests import IntegrationTestCase + +from bwh_payments.services.tabby.stub import StubTabbyClient + +WEBHOOK_TOKEN = "tabby_webhook_token" + + +class TestTabbyGatewaySettings(IntegrationTestCase): + def setUp(self): + StubTabbyClient.reset() + self.settings = frappe.get_single("Tabby Gateway Settings") + self.settings.update( + { + "merchant_code": "zz_merchant", + "currency": "SAR", + "webhook_secret": WEBHOOK_TOKEN, + "webhook_ips": "", + "success_url": "https://shop.test/en/confirm", + "cancelled_url": "https://shop.test/en/cart", + "failure_url": "https://shop.test/en/cart/checkout", + } + ) + + # --- session ---------------------------------------------------------- + + def test_create_session_keys_the_request_on_the_payment_id_not_the_checkout_id(self): + session = self.settings.create_session(12.345, "KWD", reference="GPR-0001") + + self.assertIn(session.session_id, StubTabbyClient.payments) + self.assertEqual(session.redirect_url, f"https://checkout.tabby.test/{session.session_id}") + self.assertIn("reference_id=GPR-0001", session.success_url) + # KWD carries three decimals; a plain str(float) would under-specify it. + self.assertEqual(StubTabbyClient.created_checkouts[-1]["payment"]["amount"], "12.345") + + def test_a_currency_tabby_does_not_underwrite_is_refused_before_any_call(self): + with self.assertRaises(frappe.ValidationError): + self.settings.create_session(100, "INR", reference="GPR-0002") + + self.assertEqual(StubTabbyClient.created_checkouts, []) + + def test_a_declined_shopper_is_told_why_in_words_they_can_act_on(self): + StubTabbyClient.reject_next_checkout("order_amount_too_high") + + with self.assertRaises(frappe.ValidationError) as raised: + self.settings.create_session(100, "SAR", reference="GPR-0003") + + self.assertIn("above Tabby's limit", str(raised.exception)) + + # --- status and capture ----------------------------------------------- + + def test_reading_an_authorised_payment_captures_it_for_the_authorised_amount(self): + payment_id = StubTabbyClient.add_payment(amount="250.5", currency="SAR") + StubTabbyClient.authorise(payment_id) + + self.assertEqual(self.settings.get_payment_status(payment_id), "Paid") + self.assertEqual(StubTabbyClient.captures, [{"payment_id": payment_id, "amount": "250.50"}]) + + def test_a_payment_still_being_decided_is_pending_and_not_captured(self): + payment_id = StubTabbyClient.add_payment(status="created") + + self.assertEqual(self.settings.get_payment_status(payment_id), "Pending") + self.assertEqual(StubTabbyClient.captures, []) + + def test_a_rejected_payment_is_not_paid(self): + payment_id = StubTabbyClient.add_payment(status="rejected") + + self.assertEqual(self.settings.get_payment_status(payment_id), "Not Paid") + + # --- refund ----------------------------------------------------------- + + def test_a_captured_payment_refunds_and_reports_tabbys_refund_id(self): + payment_id = StubTabbyClient.add_payment(status="closed") + + refund = self.settings.refund_payment(payment_id, 12.345, "KWD") + + self.assertEqual(StubTabbyClient.refunds, [{"payment_id": payment_id, "amount": "12.345"}]) + self.assertEqual(refund.refund_id, StubTabbyClient.payments[payment_id].refunds[-1].id) + self.assertEqual(refund.status, "succeeded") + + def test_an_uncaptured_payment_is_never_refunded(self): + payment_id = StubTabbyClient.add_payment() + StubTabbyClient.authorise(payment_id) + + with self.assertRaises(frappe.ValidationError): + self.settings.refund_payment(payment_id, 10, "SAR") + + self.assertEqual(StubTabbyClient.refunds, []) + + # --- webhook ---------------------------------------------------------- + + def deliver(self, token: str | None, request_ip: str = "127.0.0.1"): + self.addCleanup(setattr, frappe.local, "request_ip", getattr(frappe.local, "request_ip", None)) + frappe.local.request_ip = request_ip + payload = json.dumps( + {"id": "pay_tabby_hook", "status": "closed", "updated_at": "2026-10-07"} + ).encode() + headers = {"X-Webhook-Signature": token} if token is not None else {} + return self.settings.handle_webhook(payload, headers) + + def test_a_delivery_carrying_the_registered_token_is_mapped(self): + event = self.deliver(WEBHOOK_TOKEN) + + self.assertEqual(event.session_id, "pay_tabby_hook") + self.assertEqual(event.status, "Paid") + self.assertTrue(event.event_id) + + def test_a_wrong_or_missing_token_is_rejected(self): + with self.assertRaises(frappe.ValidationError): + self.deliver("guess") + with self.assertRaises(frappe.ValidationError): + self.deliver(None) + + def test_a_delivery_from_outside_the_allowlist_is_rejected_even_with_the_token(self): + self.settings.webhook_ips = "203.0.113.10" + + with self.assertRaises(frappe.ValidationError): + self.deliver(WEBHOOK_TOKEN, request_ip="198.51.100.7") diff --git a/bwh_payments/bwh_payments/doctype/telr_gateway_settings/telr_gateway_settings.py b/bwh_payments/bwh_payments/doctype/telr_gateway_settings/telr_gateway_settings.py index 657a68c..d7d6441 100644 --- a/bwh_payments/bwh_payments/doctype/telr_gateway_settings/telr_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/telr_gateway_settings/telr_gateway_settings.py @@ -1,23 +1,19 @@ # Copyright (c) 2026, Build With Hussain and contributors # For license information, please see license.txt -import base64 -import xml.etree.ElementTree as ElementTree from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit import frappe from frappe import _ -from frappe.integrations.utils import create_request_log, make_get_request, make_post_request from frappe.model.document import Document from frappe.utils.data import flt -from bwh_payments.base_class import PaymentGatewayBase +from bwh_payments.base_class import CheckoutSession, PaymentGatewayBase, RefundResult from bwh_payments.bwh_payments.utils import get_localised_url from bwh_payments.currency import get_minor_unit_exponent, validate_transaction_currency - -# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Telr call holds a worker; -# revisit if Telr latency ever shows up in the request log. -TELR_BASE_URL = "https://secure.telr.com" +from bwh_payments.services.telr.base import BaseTelrClient +from bwh_payments.services.telr.live import LiveTelrClient +from bwh_payments.services.telr.stub import StubTelrClient # Telr reports the outcome as free text. Anything unrecognised stays Pending: never terminal, and never # Paid, so an unmapped Telr state can neither release goods nor cancel a live order. @@ -35,6 +31,7 @@ # forever; an authorise-only store settles later, so calling them Paid ships goods against uncaptured # funds. Hence the per-store switch, defaulting to the safe side. TELR_AUTHORISED_STATUSES = ("authorised", "authorized") +TELR_ACCEPTED_STATUS = "A" class TelrGatewaySettings(Document, PaymentGatewayBase): @@ -61,21 +58,15 @@ class TelrGatewaySettings(Document, PaymentGatewayBase): def get_gateway_name(self) -> str: return "Telr" - def get_basic_token(self) -> str: - credentials = f"{self.store_id}:{self.get_password('auth_key')}" - return base64.b64encode(credentials.encode()).decode() + def get_client(self) -> BaseTelrClient: + if frappe.in_test: + return StubTelrClient() + + return LiveTelrClient(self.store_id, self.get_password("auth_key"), bool(self.test_mode)) @frappe.whitelist() def get_account_information(self): - endpoint = f"{TELR_BASE_URL}/api/v1/accounts" - headers = {"accept": "application/json", "authorization": f"Basic {self.get_basic_token()}"} - try: - output = make_get_request(endpoint, headers=headers) - except Exception as exception: - self.log_request(endpoint, error=exception) - raise - self.log_request(endpoint, output=output) - return output + return self.get_client().get_account_information() def create_session( self, @@ -83,40 +74,33 @@ def create_session( currency: str, reference: str | None = None, customer: dict | None = None, - ) -> dict: + ) -> CheckoutSession: currency = currency or self.currency validate_transaction_currency(currency) - endpoint = f"{TELR_BASE_URL}/gateway/order.json" - payload = { - "method": "create", - "store": self.store_id, - "authkey": self.get_password("auth_key"), - "framed": 0, - "order": { + return_urls = { + "authorised": self.build_return_url(self.authorised_url, reference), + "declined": get_localised_url(self.declined_url), + "cancelled": get_localised_url(self.cancelled_url), + } + order = self.get_client().create_order( + { "cartid": reference, "test": "1" if self.test_mode else "0", - "amount": str(flt(amount, get_minor_unit_exponent(currency))), + "amount": format_telr_amount(amount, currency), "currency": currency, "description": _("Online order payment"), }, - "return": { - "authorised": self.build_return_url(self.authorised_url, reference), - "declined": get_localised_url(self.declined_url), - "cancelled": get_localised_url(self.cancelled_url), - }, - "customer": customer or {}, - } - - response = self.post(endpoint, payload) - order = response.get("order") or {} - return { - "session_id": order.get("ref"), - "redirect_url": order.get("url"), - "success_url": payload["return"]["authorised"], - "cancel_url": payload["return"]["cancelled"], - "failure_url": payload["return"]["declined"], - } + return_urls, + customer or {}, + ) + return CheckoutSession( + session_id=order.ref, + redirect_url=order.url, + success_url=return_urls["authorised"], + cancel_url=return_urls["cancelled"], + failure_url=return_urls["declined"], + ) def build_return_url(self, url: str, reference: str | None) -> str: """Telr cannot echo its own order ref back, so the shopper returns with our request name.""" @@ -127,132 +111,44 @@ def build_return_url(self, url: str, reference: str | None) -> str: query.append(("reference_id", reference or "")) return urlunsplit((parts.scheme, parts.netloc, parts.path, urlencode(query), parts.fragment)) - def get_order(self, session_id: str) -> dict: - endpoint = f"{TELR_BASE_URL}/gateway/order.json" - payload = { - "method": "check", - "store": self.store_id, - "authkey": self.get_password("auth_key"), - "order": {"ref": session_id}, - } - return (self.post(endpoint, payload).get("order")) or {} - def get_payment_status(self, session_id: str) -> str: - order = self.get_order(session_id) - status_text = ((order.get("status") or {}).get("text") or "").strip().casefold() + order = self.get_client().check_order(session_id) + status_text = ((order.status.text if order.status else None) or "").strip().casefold() if status_text in TELR_AUTHORISED_STATUSES: return "Paid" if self.treat_authorised_as_paid else "Pending" return TELR_STATUS_MAP.get(status_text, "Pending") - def get_transaction_reference(self, session_id: str) -> str: - transaction = self.get_order(session_id).get("transaction") or {} - reference = transaction.get("ref") - if not reference: + def get_transaction_reference(self, client: BaseTelrClient, session_id: str) -> str: + transaction = client.check_order(session_id).transaction + if not (transaction and transaction.ref): frappe.throw(_("Telr has no settled transaction for this order; it cannot be refunded.")) - return reference + return transaction.ref - def handle_webhook(self, payload: bytes, headers: dict) -> dict: + def handle_webhook(self, payload: bytes, headers: dict) -> None: # Telr has no signed webhook — status is only ever taken from the authenticated `check` API in # get_payment_status. Ignoring the delivery is what keeps an unverified caller from marking a # request Paid. - return {} + return None - def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> dict: + def refund_payment(self, session_id: str, amount: float, currency: str | None = None) -> RefundResult: currency = currency or self.currency remote_auth_key = self.get_password("remote_auth_key") if not remote_auth_key: frappe.throw(_("Please set the Remote Auth Key in Telr Gateway Settings to issue refunds")) - endpoint = f"{TELR_BASE_URL}/gateway/remote.xml" - request_body = self.build_refund_request( - self.get_transaction_reference(session_id), amount, currency, remote_auth_key + client = self.get_client() + result = client.refund( + self.get_transaction_reference(client, session_id), + format_telr_amount(amount, currency), + currency, + remote_auth_key, ) - headers = {"Content-Type": "application/xml", "Accept": "application/xml"} - - try: - response = make_post_request(endpoint, data=request_body, headers=headers) - except Exception as exception: - self.log_request(endpoint, error=exception) - raise - - status, message, refund_id = read_telr_auth_response(response) - if status != "A": - self.log_request(endpoint, output={"status": status}, error=message) - frappe.throw(_("Telr refused the refund: {0}").format(message or _("unknown error"))) - - self.log_request(endpoint, output={"status": status, "refund_id": refund_id}) - return {"refund_id": refund_id, "status": "succeeded", "amount": flt(amount)} - - def build_refund_request( - self, transaction_reference: str, amount: float, currency: str, remote_auth_key: str - ) -> str: - remote = ElementTree.Element("remote") - ElementTree.SubElement(remote, "store").text = str(self.store_id) - ElementTree.SubElement(remote, "key").text = remote_auth_key - transaction = ElementTree.SubElement(remote, "tran") - ElementTree.SubElement(transaction, "type").text = "refund" - ElementTree.SubElement(transaction, "class").text = "ecom" - ElementTree.SubElement(transaction, "description").text = "Order refund" - ElementTree.SubElement(transaction, "test").text = "1" if self.test_mode else "0" - ElementTree.SubElement(transaction, "currency").text = currency - ElementTree.SubElement(transaction, "amount").text = str( - flt(amount, get_minor_unit_exponent(currency)) - ) - ElementTree.SubElement(transaction, "ref").text = transaction_reference - return ElementTree.tostring(remote, encoding="unicode") - - def post(self, endpoint: str, payload: dict) -> dict: - headers = {"accept": "application/json", "Content-Type": "application/json"} - try: - response = make_post_request(endpoint, json=payload, headers=headers) - except Exception as exception: - self.log_request(endpoint, error=exception) - raise - - # Telr answers 200 on failure too, so the body has to be inspected. - if error := response.get("error"): - self.log_request(endpoint, error=error.get("note") or error.get("message")) - frappe.throw(_("Telr rejected the request: {0}").format(error.get("message"))) - - self.log_request(endpoint, output=summarise_telr_order(response)) - return response - - def log_request(self, endpoint: str, output=None, error=None): - # The request payload carries the store auth key, so only the endpoint and the outcome are logged. - create_request_log( - {"endpoint": endpoint}, - service_name="Telr", - is_remote_request=True, - reference_doctype=self.doctype, - reference_docname=self.name, - output=output, - error=error, - status="Failed" if error else "Completed", - ) - - -def summarise_telr_order(response) -> dict: - """Telr's order payloads echo the shopper's name, email and address, so only the ids are logged.""" - order = (response or {}).get("order") or {} - return {"order_ref": order.get("ref"), "status": (order.get("status") or {}).get("text")} - - -def read_telr_auth_response(response) -> tuple[str | None, str | None, str | None]: - """Pull status, message and transaction ref out of Telr's remote.xml reply, shape-tolerantly.""" - # ponytail: stdlib ElementTree is not hardened against XML entity attacks; the response comes from a - # pinned Telr host over TLS. Move to defusedxml if Telr is ever fronted by a customer-controlled host. - body = response if isinstance(response, str) else getattr(response, "text", "") or "" - try: - root = ElementTree.fromstring(body) - except ElementTree.ParseError: - return None, _("Telr returned an unreadable response"), None + if result.status != TELR_ACCEPTED_STATUS: + frappe.throw(_("Telr refused the refund: {0}").format(result.message or _("unknown error"))) - auth = root.find("auth") - if auth is None: - return None, _("Telr returned no authorisation block"), None + return RefundResult(refund_id=result.transaction_ref, status="succeeded", amount=flt(amount)) - def text_of(tag): - node = auth.find(tag) - return node.text if node is not None else None - return text_of("status"), text_of("message"), text_of("tranref") +def format_telr_amount(amount: float, currency: str) -> str: + """Telr bills in major units, rounded to the currency's own minor unit.""" + return str(flt(amount, get_minor_unit_exponent(currency))) diff --git a/bwh_payments/bwh_payments/doctype/telr_gateway_settings/test_telr_gateway_settings.py b/bwh_payments/bwh_payments/doctype/telr_gateway_settings/test_telr_gateway_settings.py index 734bbff..d30ec1c 100644 --- a/bwh_payments/bwh_payments/doctype/telr_gateway_settings/test_telr_gateway_settings.py +++ b/bwh_payments/bwh_payments/doctype/telr_gateway_settings/test_telr_gateway_settings.py @@ -1,37 +1,91 @@ # Copyright (c) 2026, Build With Hussain and contributors # See license.txt -from unittest.mock import patch - import frappe from frappe.tests import IntegrationTestCase -AUTHORISED_ORDER = {"status": {"text": "Authorised"}} +from bwh_payments.services.telr.base import TelrOrder +from bwh_payments.services.telr.stub import StubTelrClient class TestTelrGatewaySettings(IntegrationTestCase): - def get_payment_status(self, order, treat_authorised_as_paid): + def setUp(self): + StubTelrClient.reset() + + def get_payment_status(self, status_text, treat_authorised_as_paid): settings = frappe.get_single("Telr Gateway Settings") settings.treat_authorised_as_paid = treat_authorised_as_paid - with patch.object(type(settings), "get_order", return_value=order): - return settings.get_payment_status("telr_order_ref") + return settings.get_payment_status(StubTelrClient.add_order(status_text)) def test_an_authorised_order_stays_pending_until_the_store_says_otherwise(self): """An authorise-only store settles later; calling this Paid ships goods against uncaptured funds.""" - self.assertEqual(self.get_payment_status(AUTHORISED_ORDER, 0), "Pending") + self.assertEqual(self.get_payment_status("Authorised", 0), "Pending") def test_an_authorised_order_counts_as_paid_once_the_store_is_marked_auth_and_capture(self): """Telr `ecom` authorises and captures together, so these orders otherwise sit unfulfilled forever.""" - self.assertEqual(self.get_payment_status(AUTHORISED_ORDER, 1), "Paid") + self.assertEqual(self.get_payment_status("Authorised", 1), "Paid") def test_the_american_spelling_is_mapped_the_same_way(self): - order = {"status": {"text": "authorized"}} - - self.assertEqual(self.get_payment_status(order, 1), "Paid") - self.assertEqual(self.get_payment_status(order, 0), "Pending") + self.assertEqual(self.get_payment_status("authorized", 1), "Paid") + self.assertEqual(self.get_payment_status("authorized", 0), "Pending") def test_the_switch_never_promotes_a_status_telr_did_not_authorise(self): - self.assertEqual(self.get_payment_status({"status": {"text": "Declined"}}, 1), "Not Paid") - self.assertEqual(self.get_payment_status({"status": {"text": "Cancelled"}}, 1), "Cancelled") + self.assertEqual(self.get_payment_status("Declined", 1), "Not Paid") + self.assertEqual(self.get_payment_status("Cancelled", 1), "Cancelled") # Anything unmapped stays Pending: never terminal, and never Paid. - self.assertEqual(self.get_payment_status({"status": {"text": "Who Knows"}}, 1), "Pending") + self.assertEqual(self.get_payment_status("Who Knows", 1), "Pending") + + # --- session ---------------------------------------------------------- + + def test_create_session_bills_major_units_and_brings_the_shopper_back_with_our_reference(self): + settings = frappe.get_single("Telr Gateway Settings") + settings.authorised_url = "https://shop.test/en/confirm?ref=abc" + + session = settings.create_session(12.345, "KWD", reference="GPR-0001") + + self.assertEqual(StubTelrClient.created_orders[-1]["order"]["amount"], "12.345") + self.assertEqual(StubTelrClient.created_orders[-1]["order"]["cartid"], "GPR-0001") + self.assertIn(session.session_id, StubTelrClient.orders) + self.assertIn("ref=abc", session.success_url) + self.assertIn("reference_id=GPR-0001", session.success_url) + + # --- refund ----------------------------------------------------------- + + def get_settings_with_remote_key(self): + settings = frappe.get_single("Telr Gateway Settings") + settings.remote_auth_key = "telr_remote_key" + return settings + + def test_a_refund_goes_against_the_settled_transaction_at_the_currency_precision(self): + order_ref = StubTelrClient.add_order() + transaction_ref = StubTelrClient.pay(order_ref) + + refund = self.get_settings_with_remote_key().refund_payment(order_ref, 12.3456, "KWD") + + self.assertEqual( + StubTelrClient.refunds, [{"ref": transaction_ref, "amount": "12.346", "currency": "KWD"}] + ) + self.assertEqual(refund.status, "succeeded") + self.assertEqual(refund.amount, 12.3456) + + def test_a_refund_telr_declines_throws(self): + order_ref = StubTelrClient.add_order() + StubTelrClient.pay(order_ref) + StubTelrClient.next_refund_status = "D" + + with self.assertRaises(frappe.ValidationError): + self.get_settings_with_remote_key().refund_payment(order_ref, 10, "AED") + + def test_an_order_with_no_settled_transaction_is_never_refunded(self): + order_ref = StubTelrClient.add_order("Authorised") + + with self.assertRaises(frappe.ValidationError): + self.get_settings_with_remote_key().refund_payment(order_ref, 10, "AED") + + self.assertEqual(StubTelrClient.refunds, []) + + def test_a_numeric_order_ref_from_telr_is_read_as_text(self): + order = TelrOrder.model_validate({"ref": 1234567, "transaction": {"ref": 4002201}}) + + self.assertEqual(order.ref, "1234567") + self.assertEqual(order.transaction.ref, "4002201") diff --git a/bwh_payments/bwh_payments/webhook.py b/bwh_payments/bwh_payments/webhook.py index 7435a5a..0aaad36 100644 --- a/bwh_payments/bwh_payments/webhook.py +++ b/bwh_payments/bwh_payments/webhook.py @@ -3,9 +3,12 @@ from frappe.integrations.utils import create_request_log from frappe.rate_limiter import rate_limit +from bwh_payments.base_class import WebhookEvent + WEBHOOK_ACCEPTED = {"status": "ok"} +# nosemgrep: guest-whitelisted-method # gateways post unauthenticated; each verifier checks the signature @frappe.whitelist(allow_guest=True, methods=["POST"]) @rate_limit(limit=120, seconds=60, ip_based=True) def handle(): @@ -24,28 +27,22 @@ def handle(): headers = dict(frappe.request.headers) try: - result = frappe.get_single(profile.gateway_settings).handle_webhook(payload, headers) + event = frappe.get_single(profile.gateway_settings).handle_webhook(payload, headers) + # Inside the try: a verified delivery the gateway could not turn into an event is still a 400. + event = WebhookEvent.model_validate(event) if event else None except Exception: # The payload can carry cardholder data, so only the gateway and the traceback are recorded. frappe.log_error(title=f"{gateway} webhook verification failed") log_webhook(gateway, status="Failed") return reject() - if not result: + if not event: log_webhook(gateway, status="Completed") return WEBHOOK_ACCEPTED - session_id = result.get("session_id") - status = result.get("status") - event_id = result.get("event_id") - - if not (session_id and status): - log_webhook(gateway, event_id=event_id, status="Failed") - return WEBHOOK_ACCEPTED - - request_name = frappe.db.get_value("Gateway Payment Request", {"order_ref": session_id}, "name") + request_name = frappe.db.get_value("Gateway Payment Request", {"order_ref": event.session_id}, "name") if not request_name: - log_webhook(gateway, session_id=session_id, event_id=event_id, status="Failed") + log_webhook(gateway, session_id=event.session_id, event_id=event.event_id, status="Failed") return WEBHOOK_ACCEPTED payment_request = frappe.get_doc("Gateway Payment Request", request_name) @@ -54,11 +51,11 @@ def handle(): title=f"{gateway} webhook gateway mismatch", message=f"URL gateway: {gateway}, request gateway: {payment_request.gateway}", ) - log_webhook(gateway, session_id=session_id, event_id=event_id, status="Failed") + log_webhook(gateway, session_id=event.session_id, event_id=event.event_id, status="Failed") return reject() - apply_status_as_administrator(payment_request, status, event_id) - log_webhook(gateway, session_id=session_id, event_id=event_id, status="Completed") + apply_status_as_administrator(payment_request, event.status, event.event_id) + log_webhook(gateway, session_id=event.session_id, event_id=event.event_id, status="Completed") # A replayed delivery is still a success as far as the gateway is concerned; anything else and it # keeps retrying forever. return WEBHOOK_ACCEPTED diff --git a/bwh_payments/services/__init__.py b/bwh_payments/services/__init__.py new file mode 100644 index 0000000..b158990 --- /dev/null +++ b/bwh_payments/services/__init__.py @@ -0,0 +1,8 @@ +from pydantic import BaseModel, ConfigDict + + +class GatewayModel(BaseModel): + """Base for models that mirror a gateway's JSON. Unknown fields are ignored (pydantic's default), and a + number arrives as a string where the gateway sometimes sends ids or amounts as bare numbers.""" + + model_config = ConfigDict(coerce_numbers_to_str=True) diff --git a/bwh_payments/services/razorpay/__init__.py b/bwh_payments/services/razorpay/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bwh_payments/services/razorpay/base.py b/bwh_payments/services/razorpay/base.py new file mode 100644 index 0000000..f6a6489 --- /dev/null +++ b/bwh_payments/services/razorpay/base.py @@ -0,0 +1,45 @@ +from abc import ABC, abstractmethod + +from bwh_payments.services import GatewayModel + +# Models mirror Razorpay's JSON, keeping only the fields this app reads, so a live response parses as-is. + + +class RazorpayPayment(GatewayModel): + payment_id: str + status: str | None = None + + +class RazorpayPaymentLink(GatewayModel): + id: str + status: str | None = None + short_url: str | None = None + # Razorpay sends null rather than [] for a link nobody has paid against. + payments: list[RazorpayPayment] | None = None + + +class RazorpayRefund(GatewayModel): + # Parsed after the money has moved, so nothing here may be required: a thin echo must not strand + # the refund unrecorded and refundable again. + id: str | None = None + status: str | None = None + # Minor units, as Razorpay echoes it. + amount: int | None = None + + +class BaseRazorpayClient(ABC): + """Razorpay's Payment Links and Refunds APIs. A rejected call raises `frappe.ValidationError`.""" + + @abstractmethod + def create_payment_link(self, payload: dict) -> RazorpayPaymentLink: + """POST /payment_links with the body exactly as Razorpay documents it.""" + + @abstractmethod + def get_payment_link(self, link_id: str) -> RazorpayPaymentLink: ... + + @abstractmethod + def cancel_payment_link(self, link_id: str) -> RazorpayPaymentLink: ... + + @abstractmethod + def create_refund(self, payment_id: str, amount: int) -> RazorpayRefund: + """Refund `amount` minor units of a captured payment.""" diff --git a/bwh_payments/services/razorpay/live.py b/bwh_payments/services/razorpay/live.py new file mode 100644 index 0000000..acb6115 --- /dev/null +++ b/bwh_payments/services/razorpay/live.py @@ -0,0 +1,101 @@ +from typing import NoReturn + +import frappe +from frappe import _ +from frappe.integrations.utils import create_request_log, make_get_request, make_post_request + +from bwh_payments.services.razorpay.base import ( + BaseRazorpayClient, + RazorpayPaymentLink, + RazorpayRefund, +) + +# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Razorpay call holds a +# worker; revisit if Razorpay latency ever shows up in the request log. +RAZORPAY_BASE_URL = "https://api.razorpay.com/v1" +RAZORPAY_HEADERS = {"accept": "application/json", "Content-Type": "application/json"} +SETTINGS_DOCTYPE = "Razorpay Gateway Settings" + + +class LiveRazorpayClient(BaseRazorpayClient): + def __init__(self, key_id: str, key_secret: str): + self.auth = (key_id, key_secret) + + def create_payment_link(self, payload: dict) -> RazorpayPaymentLink: + return RazorpayPaymentLink.model_validate(self.post("/payment_links", payload)) + + def get_payment_link(self, link_id: str) -> RazorpayPaymentLink: + return RazorpayPaymentLink.model_validate(self.get(f"/payment_links/{link_id}")) + + def cancel_payment_link(self, link_id: str) -> RazorpayPaymentLink: + return RazorpayPaymentLink.model_validate(self.post(f"/payment_links/{link_id}/cancel", {})) + + def create_refund(self, payment_id: str, amount: int) -> RazorpayRefund: + return RazorpayRefund.model_validate(self.post(f"/payments/{payment_id}/refund", {"amount": amount})) + + def post(self, endpoint: str, payload: dict) -> dict: + url = f"{RAZORPAY_BASE_URL}{endpoint}" + try: + response = make_post_request(url, auth=self.auth, json=payload, headers=RAZORPAY_HEADERS) + except Exception as exception: + throw_request_error(url, exception) + return read_response(url, response) + + def get(self, endpoint: str) -> dict: + url = f"{RAZORPAY_BASE_URL}{endpoint}" + try: + response = make_get_request(url, auth=self.auth, headers=RAZORPAY_HEADERS) + except Exception as exception: + throw_request_error(url, exception) + return read_response(url, response) + + +def throw_request_error(url: str, exception: Exception) -> NoReturn: + """Razorpay explains a non-2xx in a JSON error body, so the status code alone never names it.""" + description = read_razorpay_error(exception) + log_request(url, error=description or exception) + frappe.throw(_("Razorpay rejected the request: {0}").format(description or _("unknown error"))) + + +def read_response(url: str, response) -> dict: + if not isinstance(response, dict): + log_request(url, error="Razorpay returned a non-JSON body") + frappe.throw(_("Razorpay returned an unreadable response")) + + if error := response.get("error"): + log_request(url, error=error.get("description") or error.get("code")) + frappe.throw(_("Razorpay rejected the request: {0}").format(error.get("description"))) + + log_request(url, output=summarise_razorpay_entity(response)) + return response + + +def log_request(url: str, output=None, error=None): + # The request payload carries the API key and the shopper's contact details, so only the endpoint and + # the outcome are logged. + create_request_log( + {"endpoint": url}, + service_name="Razorpay", + is_remote_request=True, + reference_doctype=SETTINGS_DOCTYPE, + reference_docname=SETTINGS_DOCTYPE, + output=output, + error=error, + status="Failed" if error else "Completed", + ) + + +def summarise_razorpay_entity(response: dict) -> dict: + """Razorpay echoes the shopper's name, email and phone number, so only the ids are logged.""" + return {"id": response.get("id"), "status": response.get("status")} + + +def read_razorpay_error(exception: Exception) -> str | None: + response = getattr(exception, "response", None) + if response is None: + return None + try: + body = response.json() + except ValueError: + return None + return ((body or {}).get("error") or {}).get("description") diff --git a/bwh_payments/services/razorpay/stub.py b/bwh_payments/services/razorpay/stub.py new file mode 100644 index 0000000..1cf3fbb --- /dev/null +++ b/bwh_payments/services/razorpay/stub.py @@ -0,0 +1,102 @@ +import hashlib +import hmac +import json +from typing import ClassVar + +import frappe +from frappe import _ + +from bwh_payments.services.razorpay.base import ( + BaseRazorpayClient, + RazorpayPayment, + RazorpayPaymentLink, + RazorpayRefund, +) + + +class StubRazorpayClient(BaseRazorpayClient): + """In-memory Razorpay, selected under tests. Drive outcomes through the classmethod controls; reset() + between tests. Recordings keep Razorpay's own request shapes.""" + + links: ClassVar[dict[str, RazorpayPaymentLink]] = {} + created_links: ClassVar[list[dict]] = [] + created_refunds: ClassVar[list[dict]] = [] + next_refund_status: ClassVar[str] = "processed" + + @classmethod + def reset(cls): + cls.links = {} + cls.created_links = [] + cls.created_refunds = [] + cls.next_refund_status = "processed" + + # --- controls ----------------------------------------------------------------------------------- + + @classmethod + def add_link(cls, status: str = "created") -> str: + """A link that exists at Razorpay without this app having created it.""" + # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. + link_id = f"plink_{frappe.generate_hash(length=12)}" + cls.links[link_id] = RazorpayPaymentLink( + id=link_id, status=status, short_url=f"https://rzp.io/i/{link_id}", payments=[] + ) + return link_id + + @classmethod + def set_status(cls, link_id: str, status: str): + cls.links[link_id].status = status + + @classmethod + def add_payment(cls, link_id: str, status: str) -> str: + payment_id = f"pay_{frappe.generate_hash(length=12)}" + cls.links[link_id].payments.append(RazorpayPayment(payment_id=payment_id, status=status)) + return payment_id + + @classmethod + def pay(cls, link_id: str) -> str: + """The shopper completes the hosted page: Razorpay captures a payment and closes the link.""" + cls.set_status(link_id, "paid") + return cls.add_payment(link_id, "captured") + + # --- API ---------------------------------------------------------------------------------------- + + def create_payment_link(self, payload: dict) -> RazorpayPaymentLink: + self.created_links.append(payload) + return self.get_payment_link(self.add_link()) + + def get_payment_link(self, link_id: str) -> RazorpayPaymentLink: + if link_id not in self.links: + frappe.throw(_("Razorpay rejected the request: {0}").format("payment link not found")) + return self.links[link_id].model_copy(deep=True) + + def cancel_payment_link(self, link_id: str) -> RazorpayPaymentLink: + link = self.get_payment_link(link_id) + if link.status != "created": + frappe.throw( + _("Razorpay rejected the request: {0}").format( + f"payment link is not cancellable in state {link.status}" + ) + ) + self.set_status(link_id, "cancelled") + return self.get_payment_link(link_id) + + def create_refund(self, payment_id: str, amount: int) -> RazorpayRefund: + self.created_refunds.append({"payment_id": payment_id, "amount": amount}) + return RazorpayRefund( + id=f"rfnd_{frappe.generate_hash(length=12)}", status=self.next_refund_status, amount=amount + ) + + +def sign_payload(payload: bytes, secret: str) -> str: + """A genuine X-Razorpay-Signature, so tests exercise signature verification instead of bypassing it.""" + return hmac.new(secret.encode(), payload, hashlib.sha256).hexdigest() + + +def build_payment_link_event(link_id: str, status: str = "paid", event: str = "payment_link.paid") -> bytes: + return json.dumps( + { + "entity": "event", + "event": event, + "payload": {"payment_link": {"entity": {"id": link_id, "status": status}}}, + } + ).encode() diff --git a/bwh_payments/services/stripe/__init__.py b/bwh_payments/services/stripe/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bwh_payments/services/stripe/base.py b/bwh_payments/services/stripe/base.py new file mode 100644 index 0000000..ebea470 --- /dev/null +++ b/bwh_payments/services/stripe/base.py @@ -0,0 +1,42 @@ +from abc import ABC, abstractmethod + +from bwh_payments.services import GatewayModel + +# Models mirror Stripe's objects, keeping only the fields this app reads. + + +class StripeCheckoutSession(GatewayModel): + id: str + url: str | None = None + status: str | None = None + payment_status: str | None = None + payment_intent: str | None = None + currency: str | None = None + success_url: str | None = None + cancel_url: str | None = None + + +class StripeRefund(GatewayModel): + # Parsed after the money has moved, so nothing here may be required: a thin echo must not strand + # the refund unrecorded and refundable again. + id: str | None = None + status: str | None = None + # Minor units, as Stripe echoes it. + amount: int | None = None + + +class BaseStripeClient(ABC): + """Stripe Checkout Sessions and Refunds. A refusal raises the SDK's own `stripe.StripeError`.""" + + @abstractmethod + def create_checkout_session(self, params: dict) -> StripeCheckoutSession: + """`checkout.sessions.create` with params exactly as Stripe documents them.""" + + @abstractmethod + def retrieve_checkout_session(self, session_id: str) -> StripeCheckoutSession: ... + + @abstractmethod + def expire_checkout_session(self, session_id: str) -> StripeCheckoutSession: ... + + @abstractmethod + def create_refund(self, params: dict) -> StripeRefund: ... diff --git a/bwh_payments/services/stripe/live.py b/bwh_payments/services/stripe/live.py new file mode 100644 index 0000000..3dbdf22 --- /dev/null +++ b/bwh_payments/services/stripe/live.py @@ -0,0 +1,27 @@ +import stripe + +from bwh_payments.services.stripe.base import BaseStripeClient, StripeCheckoutSession, StripeRefund + + +class LiveStripeClient(BaseStripeClient): + def __init__(self, api_key: str): + # A client per instance keeps two configured accounts from clobbering each other through the module + # level `stripe.api_key` that the SDK otherwise reads. + self.client = stripe.StripeClient(api_key) + + def create_checkout_session(self, params: dict) -> StripeCheckoutSession: + return parse_session(self.client.v1.checkout.sessions.create(params)) + + def retrieve_checkout_session(self, session_id: str) -> StripeCheckoutSession: + return parse_session(self.client.v1.checkout.sessions.retrieve(session_id)) + + def expire_checkout_session(self, session_id: str) -> StripeCheckoutSession: + return parse_session(self.client.v1.checkout.sessions.expire(session_id)) + + def create_refund(self, params: dict) -> StripeRefund: + return StripeRefund.model_validate(self.client.v1.refunds.create(params), from_attributes=True) + + +def parse_session(session) -> StripeCheckoutSession: + # StripeObject is attribute-based, not a dict, since SDK 15. + return StripeCheckoutSession.model_validate(session, from_attributes=True) diff --git a/bwh_payments/services/stripe/stub.py b/bwh_payments/services/stripe/stub.py new file mode 100644 index 0000000..29ad57d --- /dev/null +++ b/bwh_payments/services/stripe/stub.py @@ -0,0 +1,94 @@ +import hashlib +import hmac +import json +import time +from typing import ClassVar + +import frappe +import stripe + +from bwh_payments.services.stripe.base import BaseStripeClient, StripeCheckoutSession, StripeRefund + + +class StubStripeClient(BaseStripeClient): + """In-memory Stripe, selected under tests. Drive outcomes through the classmethod controls; reset() + between tests. Recordings keep Stripe's own request shapes.""" + + sessions: ClassVar[dict[str, StripeCheckoutSession]] = {} + created_sessions: ClassVar[list[dict]] = [] + created_refunds: ClassVar[list[dict]] = [] + next_refund_status: ClassVar[str] = "succeeded" + + @classmethod + def reset(cls): + cls.sessions = {} + cls.created_sessions = [] + cls.created_refunds = [] + cls.next_refund_status = "succeeded" + + # --- controls ----------------------------------------------------------------------------------- + + @classmethod + def pay(cls, session_id: str): + """The shopper completes Checkout: the session closes with a captured PaymentIntent behind it.""" + session = cls.sessions[session_id] + session.payment_status = "paid" + session.status = "complete" + session.payment_intent = f"pi_{session_id}" + + # --- API ---------------------------------------------------------------------------------------- + + def create_checkout_session(self, params: dict) -> StripeCheckoutSession: + self.created_sessions.append(params) + # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. + session_id = f"cs_test_{frappe.generate_hash(length=12)}" + self.sessions[session_id] = StripeCheckoutSession( + id=session_id, + url=f"https://checkout.stripe.test/{session_id}", + status="open", + payment_status="unpaid", + currency=params["line_items"][0]["price_data"]["currency"], + success_url=params["success_url"], + cancel_url=params["cancel_url"], + ) + return self.retrieve_checkout_session(session_id) + + def retrieve_checkout_session(self, session_id: str) -> StripeCheckoutSession: + if session_id not in self.sessions: + raise stripe.InvalidRequestError(f"No such checkout.session: '{session_id}'", param="session") + return self.sessions[session_id].model_copy() + + def expire_checkout_session(self, session_id: str) -> StripeCheckoutSession: + session = self.retrieve_checkout_session(session_id) + if session.status != "open": + raise stripe.InvalidRequestError( + f"session is {session.status} and cannot be expired", param="session" + ) + self.sessions[session_id].status = "expired" + return self.retrieve_checkout_session(session_id) + + def create_refund(self, params: dict) -> StripeRefund: + self.created_refunds.append(params) + return StripeRefund( + id=f"re_test_{frappe.generate_hash(length=12)}", + status=self.next_refund_status, + amount=params["amount"], + ) + + +def sign_payload(payload: bytes, secret: str, timestamp: int | None = None) -> str: + """A genuine Stripe-Signature header, so tests exercise signature verification instead of bypassing it.""" + timestamp = timestamp or int(time.time()) + signed_payload = b"%d." % timestamp + payload + signature = hmac.new(secret.encode(), signed_payload, hashlib.sha256).hexdigest() + return f"t={timestamp},v1={signature}" + + +def build_checkout_completed_event(session_id: str, event_id: str = "evt_test_0001") -> bytes: + return json.dumps( + { + "id": event_id, + "type": "checkout.session.completed", + "data": {"object": {"id": session_id, "payment_status": "paid"}}, + } + ).encode() diff --git a/bwh_payments/services/tabby/__init__.py b/bwh_payments/services/tabby/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bwh_payments/services/tabby/base.py b/bwh_payments/services/tabby/base.py new file mode 100644 index 0000000..2ddeb7b --- /dev/null +++ b/bwh_payments/services/tabby/base.py @@ -0,0 +1,66 @@ +from abc import ABC, abstractmethod + +from bwh_payments.services import GatewayModel + +# Models mirror Tabby's JSON, keeping only the fields this app reads. Tabby leaves blocks out or sends +# null freely, so every field is optional and the controller decides what a missing one means. + + +class TabbyRefund(GatewayModel): + id: str | None = None + + +class TabbyPayment(GatewayModel): + id: str | None = None + status: str | None = None + # A major-unit decimal string, though Tabby has been seen to send a bare number. + amount: str | float | None = None + currency: str | None = None + refunds: list[TabbyRefund] | None = None + + +class TabbyInstallmentPlan(GatewayModel): + web_url: str | None = None + + +class TabbyAvailableProducts(GatewayModel): + installments: list[TabbyInstallmentPlan] | None = None + + +class TabbyProductStatus(GatewayModel): + rejection_reason: str | None = None + + +class TabbyProducts(GatewayModel): + installments: TabbyProductStatus | None = None + + +class TabbyConfiguration(GatewayModel): + available_products: TabbyAvailableProducts | None = None + products: TabbyProducts | None = None + + +class TabbyCheckout(GatewayModel): + id: str | None = None + status: str | None = None + payment: TabbyPayment | None = None + configuration: TabbyConfiguration | None = None + + +class BaseTabbyClient(ABC): + """Tabby's Checkout and Payments APIs. A rejected call raises `frappe.ValidationError`.""" + + @abstractmethod + def create_checkout(self, payload: dict) -> TabbyCheckout: + """POST /api/v2/checkout with the body exactly as Tabby documents it.""" + + @abstractmethod + def get_payment(self, payment_id: str) -> TabbyPayment: ... + + @abstractmethod + def capture_payment(self, payment_id: str, amount: str) -> TabbyPayment: + """Capture `amount`, a major-unit decimal string, and return the payment Tabby answers with.""" + + @abstractmethod + def refund_payment(self, payment_id: str, amount: str) -> TabbyPayment: + """Refund `amount`, a major-unit decimal string. Tabby answers with the updated payment.""" diff --git a/bwh_payments/services/tabby/live.py b/bwh_payments/services/tabby/live.py new file mode 100644 index 0000000..49811cb --- /dev/null +++ b/bwh_payments/services/tabby/live.py @@ -0,0 +1,106 @@ +from typing import NoReturn + +import frappe +from frappe import _ +from frappe.integrations.utils import create_request_log, make_get_request, make_post_request + +from bwh_payments.services.tabby.base import BaseTabbyClient, TabbyCheckout, TabbyPayment + +# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Tabby call holds a worker; +# revisit if Tabby latency ever shows up in the request log. +TABBY_BASE_URL = "https://api.tabby.ai" +SETTINGS_DOCTYPE = "Tabby Gateway Settings" + + +class LiveTabbyClient(BaseTabbyClient): + def __init__(self, key_secret: str, merchant_code: str): + self.headers = { + "Authorization": f"Bearer {key_secret}", + "Content-Type": "application/json", + "X-Merchant-Code": merchant_code, + } + + def create_checkout(self, payload: dict) -> TabbyCheckout: + return TabbyCheckout.model_validate(self.post("/api/v2/checkout", payload)) + + def get_payment(self, payment_id: str) -> TabbyPayment: + return TabbyPayment.model_validate(self.get(f"/api/v2/payments/{payment_id}")) + + def capture_payment(self, payment_id: str, amount: str) -> TabbyPayment: + return TabbyPayment.model_validate( + self.post(f"/api/v2/payments/{payment_id}/captures", {"amount": amount}) + ) + + def refund_payment(self, payment_id: str, amount: str) -> TabbyPayment: + return TabbyPayment.model_validate( + self.post(f"/api/v2/payments/{payment_id}/refunds", {"amount": amount}) + ) + + def post(self, endpoint: str, payload: dict) -> dict: + url = f"{TABBY_BASE_URL}{endpoint}" + try: + response = make_post_request(url, json=payload, headers=self.headers) + except Exception as exception: + throw_request_error(url, exception) + return read_response(url, response) + + def get(self, endpoint: str) -> dict: + url = f"{TABBY_BASE_URL}{endpoint}" + try: + response = make_get_request(url, headers=self.headers) + except Exception as exception: + throw_request_error(url, exception) + return read_response(url, response) + + +def throw_request_error(url: str, exception: Exception) -> NoReturn: + """Tabby explains a non-2xx in a JSON error body, so the status code alone never names it.""" + description = read_tabby_error(exception) + log_request(url, error=description or exception) + frappe.throw(_("Tabby rejected the request: {0}").format(description or _("unknown error"))) + + +def read_response(url: str, response) -> dict: + if not isinstance(response, dict): + log_request(url, error="Tabby returned a non-JSON body") + frappe.throw(_("Tabby returned an unreadable response")) + + if error := response.get("errorType"): + log_request(url, error=response.get("error") or error) + frappe.throw(_("Tabby rejected the request: {0}").format(response.get("error") or error)) + + log_request(url, output=summarise_tabby_payment(response)) + return response + + +def log_request(url: str, output=None, error=None): + # The headers carry the Bearer secret and the payload carries the shopper's name, phone and address, + # so only the endpoint and the outcome are logged. + create_request_log( + {"endpoint": url}, + service_name="Tabby", + is_remote_request=True, + reference_doctype=SETTINGS_DOCTYPE, + reference_docname=SETTINGS_DOCTYPE, + output=output, + error=error, + status="Failed" if error else "Completed", + ) + + +def summarise_tabby_payment(response: dict) -> dict: + """Tabby echoes the shopper's name, phone and address, so only the ids are logged.""" + # A checkout nests the payment; every other endpoint answers with the payment itself. + payment = response.get("payment") or response + return {"id": payment.get("id"), "status": response.get("status")} + + +def read_tabby_error(exception: Exception) -> str | None: + response = getattr(exception, "response", None) + if response is None: + return None + try: + body = response.json() + except ValueError: + return None + return (body or {}).get("error") or (body or {}).get("errorType") diff --git a/bwh_payments/services/tabby/stub.py b/bwh_payments/services/tabby/stub.py new file mode 100644 index 0000000..1e98e30 --- /dev/null +++ b/bwh_payments/services/tabby/stub.py @@ -0,0 +1,106 @@ +from typing import ClassVar + +import frappe +from frappe import _ + +from bwh_payments.services.tabby.base import ( + BaseTabbyClient, + TabbyAvailableProducts, + TabbyCheckout, + TabbyConfiguration, + TabbyInstallmentPlan, + TabbyPayment, + TabbyProducts, + TabbyProductStatus, + TabbyRefund, +) + + +class StubTabbyClient(BaseTabbyClient): + """In-memory Tabby, selected under tests. Drive outcomes through the classmethod controls; reset() + between tests. Recordings keep Tabby's own request shapes.""" + + payments: ClassVar[dict[str, TabbyPayment]] = {} + created_checkouts: ClassVar[list[dict]] = [] + captures: ClassVar[list[dict]] = [] + refunds: ClassVar[list[dict]] = [] + next_rejection_reason: ClassVar[str | None] = None + + @classmethod + def reset(cls): + cls.payments = {} + cls.created_checkouts = [] + cls.captures = [] + cls.refunds = [] + cls.next_rejection_reason = None + + # --- controls ----------------------------------------------------------------------------------- + + @classmethod + def add_payment(cls, status: str = "created", amount: str = "100.00", currency: str = "SAR") -> str: + """A payment that exists at Tabby without this app having created it.""" + # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. + payment_id = f"pay_tabby_{frappe.generate_hash(length=12)}" + cls.payments[payment_id] = TabbyPayment( + id=payment_id, status=status, amount=amount, currency=currency, refunds=[] + ) + return payment_id + + @classmethod + def set_status(cls, payment_id: str, status: str): + cls.payments[payment_id].status = status + + @classmethod + def authorise(cls, payment_id: str): + """The shopper is approved for instalments; no money moves until this app captures.""" + cls.set_status(payment_id, "authorized") + + @classmethod + def reject_next_checkout(cls, reason: str): + """Tabby declines the next shopper, e.g. "order_amount_too_high".""" + cls.next_rejection_reason = reason + + # --- API ---------------------------------------------------------------------------------------- + + def create_checkout(self, payload: dict) -> TabbyCheckout: + self.created_checkouts.append(payload) + if reason := self.next_rejection_reason: + StubTabbyClient.next_rejection_reason = None + return TabbyCheckout( + status="rejected", + configuration=TabbyConfiguration( + available_products=TabbyAvailableProducts(installments=[]), + products=TabbyProducts(installments=TabbyProductStatus(rejection_reason=reason)), + ), + ) + + payment_id = self.add_payment( + amount=payload["payment"]["amount"], currency=payload["payment"]["currency"] + ) + return TabbyCheckout( + id=f"chk_{payment_id}", + status="created", + payment=self.get_payment(payment_id), + configuration=TabbyConfiguration( + available_products=TabbyAvailableProducts( + installments=[TabbyInstallmentPlan(web_url=f"https://checkout.tabby.test/{payment_id}")] + ) + ), + ) + + def get_payment(self, payment_id: str) -> TabbyPayment: + if payment_id not in self.payments: + frappe.throw(_("Tabby rejected the request: {0}").format("payment not found")) + return self.payments[payment_id].model_copy(deep=True) + + def capture_payment(self, payment_id: str, amount: str) -> TabbyPayment: + self.get_payment(payment_id) + self.captures.append({"payment_id": payment_id, "amount": amount}) + self.set_status(payment_id, "closed") + return self.get_payment(payment_id) + + def refund_payment(self, payment_id: str, amount: str) -> TabbyPayment: + self.get_payment(payment_id) + self.refunds.append({"payment_id": payment_id, "amount": amount}) + self.payments[payment_id].refunds.append(TabbyRefund(id=f"rfnd_{frappe.generate_hash(length=12)}")) + return self.get_payment(payment_id) diff --git a/bwh_payments/services/telr/__init__.py b/bwh_payments/services/telr/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/bwh_payments/services/telr/base.py b/bwh_payments/services/telr/base.py new file mode 100644 index 0000000..7cc969e --- /dev/null +++ b/bwh_payments/services/telr/base.py @@ -0,0 +1,48 @@ +from abc import ABC, abstractmethod + +from bwh_payments.services import GatewayModel + +# Models mirror Telr's JSON, keeping only the fields this app reads. + + +class TelrStatus(GatewayModel): + text: str | None = None + + +class TelrTransaction(GatewayModel): + ref: str | None = None + + +class TelrOrder(GatewayModel): + ref: str | None = None + url: str | None = None + status: TelrStatus | None = None + transaction: TelrTransaction | None = None + + +class TelrRemoteResult(GatewayModel): + """The `` block of Telr's remote.xml reply. Status "A" means accepted.""" + + status: str | None = None + message: str | None = None + transaction_ref: str | None = None + + +class BaseTelrClient(ABC): + """Telr's Hosted Payment Page and remote API. A rejected JSON call raises `frappe.ValidationError`.""" + + @abstractmethod + def create_order(self, order: dict, return_urls: dict, customer: dict) -> TelrOrder: + """`order.json` method `create`, with Telr's own `order`, `return` and `customer` blocks.""" + + @abstractmethod + def check_order(self, order_ref: str) -> TelrOrder: ... + + @abstractmethod + def refund( + self, transaction_ref: str, amount: str, currency: str, remote_auth_key: str + ) -> TelrRemoteResult: + """`remote.xml` refund of `amount`, a major-unit decimal string. Never raises on a refusal.""" + + @abstractmethod + def get_account_information(self) -> dict: ... diff --git a/bwh_payments/services/telr/live.py b/bwh_payments/services/telr/live.py new file mode 100644 index 0000000..3829066 --- /dev/null +++ b/bwh_payments/services/telr/live.py @@ -0,0 +1,151 @@ +import base64 +import xml.etree.ElementTree as ElementTree + +import frappe +from frappe import _ +from frappe.integrations.utils import create_request_log, make_get_request, make_post_request + +from bwh_payments.services.telr.base import BaseTelrClient, TelrOrder, TelrRemoteResult + +# ponytail: frappe.integrations.utils.make_request takes no timeout, so a hung Telr call holds a worker; +# revisit if Telr latency ever shows up in the request log. +TELR_BASE_URL = "https://secure.telr.com" +SETTINGS_DOCTYPE = "Telr Gateway Settings" + + +class LiveTelrClient(BaseTelrClient): + def __init__(self, store_id: str, auth_key: str, test_mode: bool): + self.store_id = store_id + self.auth_key = auth_key + self.test_mode = test_mode + + def create_order(self, order: dict, return_urls: dict, customer: dict) -> TelrOrder: + response = self.post( + { + "method": "create", + "store": self.store_id, + "authkey": self.auth_key, + "framed": 0, + "order": order, + "return": return_urls, + "customer": customer, + } + ) + return TelrOrder.model_validate(response.get("order") or {}) + + def check_order(self, order_ref: str) -> TelrOrder: + response = self.post( + {"method": "check", "store": self.store_id, "authkey": self.auth_key, "order": {"ref": order_ref}} + ) + return TelrOrder.model_validate(response.get("order") or {}) + + def refund( + self, transaction_ref: str, amount: str, currency: str, remote_auth_key: str + ) -> TelrRemoteResult: + endpoint = f"{TELR_BASE_URL}/gateway/remote.xml" + headers = {"Content-Type": "application/xml", "Accept": "application/xml"} + request_body = self.build_refund_request(transaction_ref, amount, currency, remote_auth_key) + try: + response = make_post_request(endpoint, data=request_body, headers=headers) + except Exception as exception: + log_request(endpoint, error=exception) + raise + + result = read_telr_auth_response(response) + if result.status == "A": + log_request(endpoint, output={"status": result.status, "refund_id": result.transaction_ref}) + else: + log_request(endpoint, output={"status": result.status}, error=result.message) + return result + + def get_account_information(self) -> dict: + endpoint = f"{TELR_BASE_URL}/api/v1/accounts" + token = base64.b64encode(f"{self.store_id}:{self.auth_key}".encode()).decode() + headers = {"accept": "application/json", "authorization": f"Basic {token}"} + try: + output = make_get_request(endpoint, headers=headers) + except Exception as exception: + log_request(endpoint, error=exception) + raise + log_request(endpoint, output=output) + return output + + def build_refund_request( + self, transaction_ref: str, amount: str, currency: str, remote_auth_key: str + ) -> str: + remote = ElementTree.Element("remote") + ElementTree.SubElement(remote, "store").text = str(self.store_id) + ElementTree.SubElement(remote, "key").text = remote_auth_key + transaction = ElementTree.SubElement(remote, "tran") + ElementTree.SubElement(transaction, "type").text = "refund" + ElementTree.SubElement(transaction, "class").text = "ecom" + ElementTree.SubElement(transaction, "description").text = "Order refund" + ElementTree.SubElement(transaction, "test").text = "1" if self.test_mode else "0" + ElementTree.SubElement(transaction, "currency").text = currency + ElementTree.SubElement(transaction, "amount").text = amount + ElementTree.SubElement(transaction, "ref").text = transaction_ref + return ElementTree.tostring(remote, encoding="unicode") + + def post(self, payload: dict) -> dict: + endpoint = f"{TELR_BASE_URL}/gateway/order.json" + headers = {"accept": "application/json", "Content-Type": "application/json"} + try: + response = make_post_request(endpoint, json=payload, headers=headers) + except Exception as exception: + log_request(endpoint, error=exception) + raise + + if not isinstance(response, dict): + log_request(endpoint, error="Telr returned a non-JSON body") + frappe.throw(_("Telr returned an unreadable response")) + + # Telr answers 200 on failure too, so the body has to be inspected. + if error := response.get("error"): + log_request(endpoint, error=error.get("note") or error.get("message")) + frappe.throw(_("Telr rejected the request: {0}").format(error.get("message"))) + + log_request(endpoint, output=summarise_telr_order(response)) + return response + + +def log_request(endpoint: str, output=None, error=None): + # The request payload carries the store auth key, so only the endpoint and the outcome are logged. + create_request_log( + {"endpoint": endpoint}, + service_name="Telr", + is_remote_request=True, + reference_doctype=SETTINGS_DOCTYPE, + reference_docname=SETTINGS_DOCTYPE, + output=output, + error=error, + status="Failed" if error else "Completed", + ) + + +def summarise_telr_order(response) -> dict: + """Telr's order payloads echo the shopper's name, email and address, so only the ids are logged.""" + order = (response or {}).get("order") or {} + return {"order_ref": order.get("ref"), "status": (order.get("status") or {}).get("text")} + + +def read_telr_auth_response(response) -> TelrRemoteResult: + """Pull status, message and transaction ref out of Telr's remote.xml reply, shape-tolerantly.""" + # ponytail: stdlib ElementTree is not hardened against XML entity attacks; the response comes from a + # pinned Telr host over TLS. Move to defusedxml if Telr is ever fronted by a customer-controlled host. + body = response if isinstance(response, str) else getattr(response, "text", "") or "" + try: + root = ElementTree.fromstring(body) + except ElementTree.ParseError: + return TelrRemoteResult(message=_("Telr returned an unreadable response")) + + auth = root.find("auth") + if auth is None: + return TelrRemoteResult(message=_("Telr returned no authorisation block")) + + def text_of(tag): + node = auth.find(tag) + return node.text if node is not None else None + + return TelrRemoteResult( + status=text_of("status"), message=text_of("message"), transaction_ref=text_of("tranref") + ) diff --git a/bwh_payments/services/telr/stub.py b/bwh_payments/services/telr/stub.py new file mode 100644 index 0000000..8c0d920 --- /dev/null +++ b/bwh_payments/services/telr/stub.py @@ -0,0 +1,77 @@ +from typing import ClassVar + +import frappe +from frappe import _ + +from bwh_payments.services.telr.base import ( + BaseTelrClient, + TelrOrder, + TelrRemoteResult, + TelrStatus, + TelrTransaction, +) + + +class StubTelrClient(BaseTelrClient): + """In-memory Telr, selected under tests. Drive outcomes through the classmethod controls; reset() + between tests. Recordings keep Telr's own request shapes.""" + + orders: ClassVar[dict[str, TelrOrder]] = {} + created_orders: ClassVar[list[dict]] = [] + refunds: ClassVar[list[dict]] = [] + next_refund_status: ClassVar[str] = "A" + + @classmethod + def reset(cls): + cls.orders = {} + cls.created_orders = [] + cls.refunds = [] + cls.next_refund_status = "A" + + # --- controls ----------------------------------------------------------------------------------- + + @classmethod + def add_order(cls, status_text: str = "Pending") -> str: + """An order that exists at Telr without this app having created it.""" + # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. + order_ref = f"telr_{frappe.generate_hash(length=12)}" + cls.orders[order_ref] = TelrOrder( + ref=order_ref, + url=f"https://secure.telr.test/gateway/process.html?o={order_ref}", + status=TelrStatus(text=status_text), + ) + return order_ref + + @classmethod + def set_status(cls, order_ref: str, status_text: str): + cls.orders[order_ref].status = TelrStatus(text=status_text) + + @classmethod + def pay(cls, order_ref: str) -> str: + """The shopper completes the Hosted Payment Page: Telr settles a transaction against the order.""" + cls.set_status(order_ref, "Paid") + transaction_ref = f"tran_{frappe.generate_hash(length=12)}" + cls.orders[order_ref].transaction = TelrTransaction(ref=transaction_ref) + return transaction_ref + + # --- API ---------------------------------------------------------------------------------------- + + def create_order(self, order: dict, return_urls: dict, customer: dict) -> TelrOrder: + self.created_orders.append({"order": order, "return": return_urls, "customer": customer}) + return self.check_order(self.add_order()) + + def check_order(self, order_ref: str) -> TelrOrder: + if order_ref not in self.orders: + frappe.throw(_("Telr rejected the request: {0}").format("Order not found")) + return self.orders[order_ref].model_copy(deep=True) + + def refund( + self, transaction_ref: str, amount: str, currency: str, remote_auth_key: str + ) -> TelrRemoteResult: + self.refunds.append({"ref": transaction_ref, "amount": amount, "currency": currency}) + if self.next_refund_status != "A": + return TelrRemoteResult(status=self.next_refund_status, message="Refund declined") + return TelrRemoteResult(status="A", message="Authorised", transaction_ref=f"rfnd_{transaction_ref}") + + def get_account_information(self) -> dict: + return {} diff --git a/bwh_payments/tests/fake_razorpay.py b/bwh_payments/tests/fake_razorpay.py deleted file mode 100644 index ee683c0..0000000 --- a/bwh_payments/tests/fake_razorpay.py +++ /dev/null @@ -1,122 +0,0 @@ -"""A stand-in for Razorpay's HTTP transport. - -Razorpay is driven over raw HTTP here, not an SDK, so the seam is the two request helpers the controller -imports at module scope. Patch these over `make_post_request` / `make_get_request` *inside* -`razorpay_gateway_settings` and the controller runs its real code — minor-unit conversion, URL building, -status mapping, signature verification — against a recording fake instead of the network. -""" - -import hashlib -import hmac -import json -from typing import ClassVar - -import frappe - -PAYMENT_LINKS_ENDPOINT = "/payment_links" - - -class FakeRazorpay: - """Records what the controller sent so a test can assert on the exact minor-unit amount.""" - - created_links: ClassVar[list[dict]] = [] - created_refunds: ClassVar[list[dict]] = [] - links: ClassVar[dict[str, dict]] = {} - next_refund_status = "processed" - - @classmethod - def reset(cls): - cls.created_links = [] - cls.created_refunds = [] - cls.links = {} - cls.next_refund_status = "processed" - - @classmethod - def register_link(cls, status: str = "created", link_id: str | None = None, **fields) -> str: - """Put a payment link on the fake in a given status and return its id.""" - # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. - link_id = link_id or f"plink_{frappe.generate_hash(length=12)}" - cls.links[link_id] = { - "id": link_id, - "status": status, - "short_url": f"https://rzp.io/i/{link_id}", - "payments": [], - **fields, - } - return link_id - - @classmethod - def add_payment(cls, link_id: str, status: str, payment_id: str | None = None) -> str: - payment_id = payment_id or f"pay_{frappe.generate_hash(length=12)}" - cls.links[link_id].setdefault("payments", []).append({"payment_id": payment_id, "status": status}) - return payment_id - - # --- transport -------------------------------------------------------- - - @classmethod - def post(cls, url, auth=None, json=None, headers=None, **kwargs): - payload = json or {} - endpoint = url.split("/v1", 1)[-1] - - if endpoint == PAYMENT_LINKS_ENDPOINT: - cls.created_links.append(payload) - link_id = cls.register_link( - status="created", amount=payload.get("amount"), currency=payload.get("currency") - ) - return dict(cls.links[link_id]) - - if endpoint.startswith(f"{PAYMENT_LINKS_ENDPOINT}/") and endpoint.endswith("/cancel"): - link_id = endpoint.split("/")[2] - link = cls.links.get(link_id) - if not link: - return {"error": {"code": "BAD_REQUEST_ERROR", "description": "payment link not found"}} - if link["status"] != "created": - return { - "error": { - "code": "BAD_REQUEST_ERROR", - "description": f"payment link is not cancellable in state {link['status']}", - } - } - link["status"] = "cancelled" - return dict(link) - - if endpoint.startswith("/payments/") and endpoint.endswith("/refund"): - cls.created_refunds.append({"payment_id": endpoint.split("/")[2], **payload}) - return { - "id": f"rfnd_{frappe.generate_hash(length=12)}", - "status": cls.next_refund_status, - # Razorpay echoes the amount back in minor units. - "amount": payload.get("amount"), - } - - raise AssertionError(f"unexpected Razorpay POST to {url}") - - @classmethod - def get(cls, url, auth=None, headers=None, **kwargs): - endpoint = url.split("/v1", 1)[-1] - - if endpoint.startswith(f"{PAYMENT_LINKS_ENDPOINT}/"): - link_id = endpoint.rsplit("/", 1)[-1] - link = cls.links.get(link_id) - if not link: - return {"error": {"code": "BAD_REQUEST_ERROR", "description": "payment link not found"}} - return dict(link) - - raise AssertionError(f"unexpected Razorpay GET to {url}") - - -def sign_razorpay_payload(payload: bytes, secret: str) -> str: - """Build a genuine X-Razorpay-Signature so signature verification is exercised, not bypassed.""" - return hmac.new(secret.encode(), payload, hashlib.sha256).hexdigest() - - -def build_payment_link_paid_event( - link_id: str, status: str = "paid", event: str = "payment_link.paid" -) -> bytes: - return json.dumps( - { - "entity": "event", - "event": event, - "payload": {"payment_link": {"entity": {"id": link_id, "status": status}}}, - } - ).encode() diff --git a/bwh_payments/tests/fake_stripe.py b/bwh_payments/tests/fake_stripe.py deleted file mode 100644 index 19da6ca..0000000 --- a/bwh_payments/tests/fake_stripe.py +++ /dev/null @@ -1,118 +0,0 @@ -"""A stand-in for the Stripe SDK transport. - -The gateway controller runs its real code — amount conversion, URL building, response mapping — against -this instead of the network, so tests exercise the shipped logic and never touch a live account. -""" - -import hashlib -import hmac -import json -import time -from typing import ClassVar - -import frappe -import stripe -from frappe import _dict - - -class FakeStripeSession(_dict): - pass - - -class FakeStripeClient: - """Records what the controller sent so a test can assert on the exact minor-unit amount.""" - - created_sessions: ClassVar[list[dict]] = [] - created_refunds: ClassVar[list[dict]] = [] - sessions: ClassVar[dict[str, dict]] = {} - next_refund_status = "succeeded" - refund_counter = 0 - - def __init__(self, api_key=None): - self.api_key = api_key - self.checkout = _dict(sessions=FakeSessionService()) - self.refunds = FakeRefundService() - - @classmethod - def reset(cls): - cls.created_sessions = [] - cls.created_refunds = [] - cls.sessions = {} - cls.next_refund_status = "succeeded" - cls.refund_counter = 0 - - @classmethod - def register_paid_session(cls, session_id: str, currency: str = "sar"): - cls.sessions[session_id] = { - "id": session_id, - "payment_status": "paid", - "status": "complete", - "payment_intent": f"pi_{session_id}", - "currency": currency, - } - - -class FakeSessionService: - def create(self, params): - FakeStripeClient.created_sessions.append(params) - # Unique per call: the doctype enforces a unique order_ref and rows outlive a single test. - session_id = f"cs_test_{frappe.generate_hash(length=12)}" - FakeStripeClient.sessions[session_id] = { - "id": session_id, - "payment_status": "unpaid", - "status": "open", - "payment_intent": None, - "currency": params["line_items"][0]["price_data"]["currency"], - } - return FakeStripeSession( - id=session_id, - url=f"https://checkout.stripe.test/{session_id}", - success_url=params["success_url"], - cancel_url=params["cancel_url"], - ) - - def retrieve(self, session_id): - session = FakeStripeClient.sessions.get(session_id) - if not session: - raise KeyError(f"unknown session {session_id}") - return FakeStripeSession(**session) - - def expire(self, session_id): - session = FakeStripeClient.sessions.get(session_id) - if not session: - raise stripe.InvalidRequestError(f"unknown session {session_id}", param="session") - if session["status"] != "open": - raise stripe.InvalidRequestError( - f"session is {session['status']} and cannot be expired", param="session" - ) - session["status"] = "expired" - return FakeStripeSession(**session) - - -class FakeRefundService: - def create(self, params): - FakeStripeClient.created_refunds.append(params) - FakeStripeClient.refund_counter += 1 - return FakeStripeSession( - id=f"re_test_{frappe.generate_hash(length=12)}", - status=FakeStripeClient.next_refund_status, - amount=params["amount"], - ) - - -def sign_stripe_payload(payload: bytes, secret: str, timestamp: int | None = None) -> str: - """Build a genuine Stripe-Signature header so signature verification is exercised, not bypassed.""" - timestamp = timestamp or int(time.time()) - signed_payload = b"%d." % timestamp + payload - signature = hmac.new(secret.encode(), signed_payload, hashlib.sha256).hexdigest() - return f"t={timestamp},v1={signature}" - - -def build_checkout_completed_event(session_id: str, event_id: str = "evt_test_0001") -> bytes: - return json.dumps( - { - "id": event_id, - "type": "checkout.session.completed", - "data": {"object": {"id": session_id, "payment_status": "paid"}}, - } - ).encode() diff --git a/bwh_payments/tests/test_razorpay_webhook.py b/bwh_payments/tests/test_razorpay_webhook.py index 8f3e282..15406bf 100644 --- a/bwh_payments/tests/test_razorpay_webhook.py +++ b/bwh_payments/tests/test_razorpay_webhook.py @@ -12,7 +12,7 @@ RazorpayTestCase, make_razorpay_payment_request, ) -from bwh_payments.tests.fake_razorpay import build_payment_link_paid_event, sign_razorpay_payload +from bwh_payments.services.razorpay.stub import build_payment_link_event, sign_payload # No IGNORE_TEST_RECORD_DEPENDENCIES here: frappe only honours it inside a doctype folder and raises # NotImplementedError otherwise. Outside one it loads no test records at all, which is what we want. @@ -50,9 +50,9 @@ def get_status_code(self): def test_a_correctly_signed_event_marks_the_request_paid(self): payment_request = make_razorpay_payment_request(100, "INR") - payload = build_payment_link_paid_event(payment_request.order_ref) + payload = build_payment_link_event(payment_request.order_ref) - response = self.post_webhook(payload, sign_razorpay_payload(payload, RAZORPAY_WEBHOOK_SECRET)) + response = self.post_webhook(payload, sign_payload(payload, RAZORPAY_WEBHOOK_SECRET)) self.assertEqual(response["status"], "ok") payment_request.reload() @@ -60,9 +60,9 @@ def test_a_correctly_signed_event_marks_the_request_paid(self): def test_a_forged_signature_is_rejected_and_nothing_is_marked_paid(self): payment_request = make_razorpay_payment_request(100, "INR") - payload = build_payment_link_paid_event(payment_request.order_ref) + payload = build_payment_link_event(payment_request.order_ref) - response = self.post_webhook(payload, sign_razorpay_payload(payload, "attacker_guess")) + response = self.post_webhook(payload, sign_payload(payload, "attacker_guess")) self.assertEqual(response["status"], "error") self.assertEqual(self.get_status_code(), 400) @@ -71,7 +71,7 @@ def test_a_forged_signature_is_rejected_and_nothing_is_marked_paid(self): def test_an_unsigned_delivery_is_rejected(self): payment_request = make_razorpay_payment_request(100, "INR") - payload = build_payment_link_paid_event(payment_request.order_ref) + payload = build_payment_link_event(payment_request.order_ref) response = self.post_webhook(payload, None) @@ -82,8 +82,8 @@ def test_an_unsigned_delivery_is_rejected(self): def test_a_replayed_delivery_is_accepted_but_applied_once(self): payment_request = make_razorpay_payment_request(100, "INR") - payload = build_payment_link_paid_event(payment_request.order_ref) - signature = sign_razorpay_payload(payload, RAZORPAY_WEBHOOK_SECRET) + payload = build_payment_link_event(payment_request.order_ref) + signature = sign_payload(payload, RAZORPAY_WEBHOOK_SECRET) self.post_webhook(payload, signature, event_id="evt_rzp_replay") second = self.post_webhook(payload, signature, event_id="evt_rzp_replay") @@ -96,7 +96,7 @@ def test_a_replayed_delivery_is_accepted_but_applied_once(self): self.assertEqual(payment_request.last_webhook_event_id, "evt_rzp_replay") def test_a_rejection_does_not_echo_the_gateway_error_back_to_the_caller(self): - payload = build_payment_link_paid_event("plink_nothing") + payload = build_payment_link_event("plink_nothing") self.post_webhook(payload, None) @@ -104,9 +104,9 @@ def test_a_rejection_does_not_echo_the_gateway_error_back_to_the_caller(self): def test_the_webhook_log_never_stores_the_raw_payload(self): payment_request = make_razorpay_payment_request(100, "INR") - payload = build_payment_link_paid_event(payment_request.order_ref) + payload = build_payment_link_event(payment_request.order_ref) - self.post_webhook(payload, sign_razorpay_payload(payload, RAZORPAY_WEBHOOK_SECRET)) + self.post_webhook(payload, sign_payload(payload, RAZORPAY_WEBHOOK_SECRET)) logged = frappe.get_all( "Integration Request", diff --git a/bwh_payments/tests/test_webhook.py b/bwh_payments/tests/test_webhook.py index fe39de7..a0e7d1a 100644 --- a/bwh_payments/tests/test_webhook.py +++ b/bwh_payments/tests/test_webhook.py @@ -1,8 +1,6 @@ # Copyright (c) 2026, Build With Hussain and contributors # See license.txt -from unittest.mock import patch - import frappe from frappe.tests import IntegrationTestCase from werkzeug.test import EnvironBuilder @@ -15,22 +13,12 @@ configure_stripe_gateway, make_payment_request, ) -from bwh_payments.bwh_payments.doctype.stripe_gateway_settings import stripe_gateway_settings -from bwh_payments.tests.fake_stripe import ( - FakeStripeClient, - build_checkout_completed_event, - sign_stripe_payload, -) +from bwh_payments.services.stripe.stub import StubStripeClient, build_checkout_completed_event, sign_payload class TestWebhook(IntegrationTestCase): def setUp(self): - FakeStripeClient.reset() - self.stripe_client_patch = patch.object( - stripe_gateway_settings.stripe, "StripeClient", FakeStripeClient - ) - self.stripe_client_patch.start() - self.addCleanup(self.stripe_client_patch.stop) + StubStripeClient.reset() configure_stripe_gateway() self.original_request = getattr(frappe.local, "request", None) self.addCleanup(self.restore_request) @@ -60,9 +48,9 @@ def get_status_code(self): def test_a_correctly_signed_event_marks_the_request_paid(self): payment_request = make_payment_request(100, "SAR") payload = build_checkout_completed_event(payment_request.order_ref) - FakeStripeClient.register_paid_session(payment_request.order_ref) + StubStripeClient.pay(payment_request.order_ref) - response = self.post_webhook(GATEWAY, payload, sign_stripe_payload(payload, WEBHOOK_SECRET)) + response = self.post_webhook(GATEWAY, payload, sign_payload(payload, WEBHOOK_SECRET)) self.assertEqual(response["status"], "ok") payment_request.reload() @@ -71,7 +59,7 @@ def test_a_correctly_signed_event_marks_the_request_paid(self): def test_a_forged_signature_is_rejected_and_nothing_is_marked_paid(self): payment_request = make_payment_request(100, "SAR") payload = build_checkout_completed_event(payment_request.order_ref) - forged = sign_stripe_payload(payload, "whsec_attacker_guess") + forged = sign_payload(payload, "whsec_attacker_guess") response = self.post_webhook(GATEWAY, payload, forged) @@ -94,7 +82,7 @@ def test_an_unsigned_delivery_is_rejected(self): def test_an_unknown_gateway_is_rejected(self): payload = build_checkout_completed_event("cs_test_nothing") - response = self.post_webhook("Not A Gateway", payload, sign_stripe_payload(payload, WEBHOOK_SECRET)) + response = self.post_webhook("Not A Gateway", payload, sign_payload(payload, WEBHOOK_SECRET)) self.assertEqual(response["status"], "error") self.assertEqual(self.get_status_code(), 400) @@ -105,7 +93,7 @@ def test_every_verification_failure_answers_with_the_same_opaque_400(self): unknown_gateway = self.post_webhook("Not A Gateway", payload, None) unsigned = self.post_webhook(GATEWAY, payload, None) - forged = self.post_webhook(GATEWAY, payload, sign_stripe_payload(payload, "whsec_attacker_guess")) + forged = self.post_webhook(GATEWAY, payload, sign_payload(payload, "whsec_attacker_guess")) self.assertEqual(unknown_gateway, unsigned) self.assertEqual(unsigned, forged) @@ -122,8 +110,8 @@ def test_a_rejection_does_not_echo_the_gateway_error_back_to_the_caller(self): def test_a_replayed_delivery_is_accepted_but_applied_once(self): payment_request = make_payment_request(100, "SAR") payload = build_checkout_completed_event(payment_request.order_ref, event_id="evt_replay") - signature = sign_stripe_payload(payload, WEBHOOK_SECRET) - FakeStripeClient.register_paid_session(payment_request.order_ref) + signature = sign_payload(payload, WEBHOOK_SECRET) + StubStripeClient.pay(payment_request.order_ref) self.post_webhook(GATEWAY, payload, signature) second = self.post_webhook(GATEWAY, payload, signature) @@ -137,9 +125,9 @@ def test_a_replayed_delivery_is_accepted_but_applied_once(self): def test_the_webhook_log_never_stores_the_raw_payload(self): payment_request = make_payment_request(100, "SAR") payload = build_checkout_completed_event(payment_request.order_ref, event_id="evt_privacy") - FakeStripeClient.register_paid_session(payment_request.order_ref) + StubStripeClient.pay(payment_request.order_ref) - self.post_webhook(GATEWAY, payload, sign_stripe_payload(payload, WEBHOOK_SECRET)) + self.post_webhook(GATEWAY, payload, sign_payload(payload, WEBHOOK_SECRET)) logged = frappe.get_all( "Integration Request", diff --git a/pyproject.toml b/pyproject.toml index 5bbe0c9..cd6c3be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,8 +13,10 @@ dynamic = ["version"] dependencies = [ # "frappe~=16.0.0" # Installed and managed by bench. # stripe_gateway_settings imports this at module level, so every install of a - # dependent app dies at import without it. StripeClient needs 11 or newer. - "stripe>=11,<16", + # dependent app dies at import without it. StripeClient.v1 needs 12.5 or newer. + "stripe>=12.5,<16", + # Gateway results and service-client responses are pydantic models. Frappe pins the exact version. + "pydantic>=2,<3", ] [build-system]