From b3035a3a03365cdc2ea1917539454387f1e3c42d Mon Sep 17 00:00:00 2001 From: opensensor-agent Date: Sat, 17 Oct 2026 17:41:00 +0000 Subject: [PATCH] Harden payment endpoint authorization and state transitions (#27) Enforce a single payment access policy on all checkout endpoints (initiate/confirm/return/cancel): an authenticated submitter may act only on their own submission, and an anonymous public-form submitter needs a short-lived, unguessable, session-bound checkout credential issued when the form post enters pending_payment. Guessed numeric IDs now get a non-disclosing 404. - Initiation is idempotent per attempt: repeated GETs reuse the single open PaymentRecord and its stable idempotency key (forwarded to Stripe's PaymentIntent.create) under a row lock, so retries cannot create uncontrolled attempts or duplicate charges. A legitimate retry after cancel/failure starts a fresh key. - Confirm/return are restricted to the originating open attempt and valid lifecycle states; provider results are verified and applied without ever rolling a locked (completed/refunded) record backward. - Cancellation is a CSRF-protected POST-only action scoped to pending_payment submissions; it cancels only open attempts and can never revert a completed/submitted/approved submission to draft. Templates now use POST forms for the cancel action. - Stripe webhooks fail closed: events are rejected (HTTP 400) when STRIPE_WEBHOOK_SECRET is unset or the signature is invalid. The provider interface requires the same authenticated-event guarantee from all providers via the new WebhookVerificationError. Replayed or out-of-order events create no duplicate workflow tasks. - Regression tests cover anonymous/stranger/owner/anonymous-checkout access, guessed IDs, GET vs POST, repeated requests, missing/invalid webhook signatures, replay protection, and cancellation after completion. Docs updated for the required webhook secret and the new checkout/idempotency behavior. --- CHANGELOG.md | 24 + django_forms_workflows/payments/__init__.py | 9 +- django_forms_workflows/payments/base.py | 24 + django_forms_workflows/payments/checkout.py | 84 ++ .../payments/stripe_provider.py | 51 +- django_forms_workflows/payments/views.py | 296 ++++-- .../payment_collect.html | 10 +- .../django_forms_workflows/payment_error.html | 9 +- django_forms_workflows/views.py | 10 + docs/PAYMENTS.md | 87 +- tests/test_payment_security.py | 848 ++++++++++++++++++ 11 files changed, 1341 insertions(+), 111 deletions(-) create mode 100644 django_forms_workflows/payments/checkout.py create mode 100644 tests/test_payment_security.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ee4ccc..af6a236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Security +- **Payment endpoints hardened against unauthorized access and unsafe state + transitions (issue #27, supersedes #4).** All checkout endpoints + (`initiate`, `confirm`, `return`, `cancel`) now enforce one access policy + before touching records or providers: an authenticated submitter may act + only on their own submission, and an anonymous public-form submitter needs + a short-lived (`FORMS_WORKFLOWS_PAYMENT_CHECKOUT_TTL`, default 1 hour), + unguessable session-bound checkout credential issued when the form post + enters `pending_payment`. Guessing a numeric submission or payment-record + ID now yields a non-disclosing 404. Payment initiation is idempotent per + attempt — repeated GETs reuse the single open `PaymentRecord` and its + stable idempotency key (now forwarded to Stripe's `PaymentIntent.create`) + instead of creating uncontrolled attempts; a legitimate retry after + cancel/failure starts a fresh key. Cancellation is a CSRF-protected + `POST`-only action restricted to `pending_payment` submissions and can no + longer revert a completed/submitted/approved submission to `draft`. + Stripe webhooks now **fail closed**: events are rejected (HTTP 400) when + `STRIPE_WEBHOOK_SECRET` is unset or the signature is invalid, and the + provider interface requires the same authenticated-event guarantee from + all providers via `WebhookVerificationError`. Replayed or out-of-order + events no longer roll payment state backward (`completed`/`refunded` are + terminal; only `completed → refunded` is allowed) and duplicate deliveries + create no duplicate workflow tasks. `payment_collect.html` and + `payment_error.html` cancel actions are now POST forms. + - **`social-auth-core` 4.8.3 → 5.1.1 in `poetry.lock`.** Closes five Dependabot advisories against the locked version: GHSA-vqg6-3fw6-j9jg (session fixation), GHSA-3c93-f73f-qc9h (VK App backend accepted unsigned callback data), diff --git a/django_forms_workflows/payments/__init__.py b/django_forms_workflows/payments/__init__.py index 3bf433e..c382296 100644 --- a/django_forms_workflows/payments/__init__.py +++ b/django_forms_workflows/payments/__init__.py @@ -12,7 +12,13 @@ without modifying the library itself. """ -from .base import PaymentFlow, PaymentProvider, PaymentResult, PaymentStatus +from .base import ( + PaymentFlow, + PaymentProvider, + PaymentResult, + PaymentStatus, + WebhookVerificationError, +) from .registry import ( clear, get_available_providers, @@ -27,6 +33,7 @@ "PaymentProvider", "PaymentResult", "PaymentStatus", + "WebhookVerificationError", "clear", "get_available_providers", "get_provider", diff --git a/django_forms_workflows/payments/base.py b/django_forms_workflows/payments/base.py index 374974c..f9ffd96 100644 --- a/django_forms_workflows/payments/base.py +++ b/django_forms_workflows/payments/base.py @@ -47,6 +47,17 @@ class PaymentStatus(Enum): REFUNDED = "refunded" +class WebhookVerificationError(Exception): + """ + Raised by a provider's ``handle_webhook()`` when the event payload + cannot be authenticated (e.g. a missing/invalid signature, or a + required verification secret that is not configured). + + Providers must raise this instead of processing unverified payloads: + the webhook endpoint fails closed and discards the event. + """ + + @dataclass class PaymentResult: """Returned by create_payment() and confirm_payment().""" @@ -93,12 +104,19 @@ def create_payment( submission_id: int, description: str = "", metadata: dict[str, Any] | None = None, + idempotency_key: str | None = None, ) -> PaymentResult: """ Initiate a payment. For INLINE providers: creates a payment intent, returns client_secret. For REDIRECT providers: creates a session, returns redirect_url. + + ``idempotency_key`` is stable for a single payment attempt (one + ``PaymentRecord``). Providers that support request idempotency + should forward it so a retried initiation cannot create duplicate + provider-side charges. A new attempt (after a cancellation or + failure) is issued a fresh key. """ @abstractmethod @@ -121,6 +139,12 @@ def handle_webhook(self, request: Any) -> PaymentResult | None: Override if the provider sends server-to-server callbacks. Returns None if the event is not relevant. + + Implementations MUST authenticate the event before returning a + result (e.g. verify a signature or shared secret) and raise + :class:`WebhookVerificationError` when verification fails or the + required verification secret is not configured. Unverified + payloads must never produce a state-changing result. """ return None diff --git a/django_forms_workflows/payments/checkout.py b/django_forms_workflows/payments/checkout.py new file mode 100644 index 0000000..bfa5814 --- /dev/null +++ b/django_forms_workflows/payments/checkout.py @@ -0,0 +1,84 @@ +""" +Checkout authorization for the payment flow. + +Payment endpoints act on a :class:`~django_forms_workflows.models.FormSubmission` +that is in ``pending_payment``. Access is governed by exactly one policy: + +* **Owned submissions** — only the authenticated submitter may act on them. + A numeric submission or payment-record ID alone grants nothing. +* **Anonymous (public-form) submissions** — the browser that just posted the + public form is issued a *checkout credential*: an unguessable token stored + in its session and bound to the submission ID, valid for a short time + (``FORMS_WORKFLOWS_PAYMENT_CHECKOUT_TTL`` seconds, default one hour). + +The credential is granted by the submit views when a submission enters +``pending_payment`` and is revoked when the checkout is cancelled. It is +never exposed in a URL. Callers that fail the policy receive a +non-disclosing 404, identical to a nonexistent submission. +""" + +from __future__ import annotations + +import secrets +import time + +from django.conf import settings + +SESSION_KEY = "payment_checkouts" +DEFAULT_CHECKOUT_TTL_SECONDS = 3600 # 1 hour + + +def _checkout_ttl() -> int: + return int( + getattr( + settings, + "FORMS_WORKFLOWS_PAYMENT_CHECKOUT_TTL", + DEFAULT_CHECKOUT_TTL_SECONDS, + ) + ) + + +def grant_checkout_access(request, submission) -> None: + """Bind a short-lived checkout credential for *submission* to the session. + + Called by the submit views right after a submission enters + ``pending_payment`` so the same browser session can complete (confirm, + return, or cancel) the checkout that belongs to it. + """ + checkouts = dict(request.session.get(SESSION_KEY, {})) + checkouts[str(submission.id)] = { + "token": secrets.token_urlsafe(32), + "exp": time.time() + _checkout_ttl(), + } + request.session[SESSION_KEY] = checkouts + request.session.modified = True + + +def revoke_checkout_access(request, submission) -> None: + """Drop any checkout credential for *submission* from the session.""" + checkouts = dict(request.session.get(SESSION_KEY, {})) + if checkouts.pop(str(submission.id), None) is not None: + request.session[SESSION_KEY] = checkouts + request.session.modified = True + + +def has_checkout_access(request, submission) -> bool: + """The single payment access policy used by every payment endpoint. + + * Authenticated submitter: may act only on their own submission. + * Anonymous submission (``submitter`` is null): the caller's session + must hold an unexpired checkout credential issued by + :func:`grant_checkout_access`. + """ + if submission.submitter_id is not None: + return ( + request.user.is_authenticated and request.user.pk == submission.submitter_id + ) + + entry = request.session.get(SESSION_KEY, {}).get(str(submission.id)) + if not entry: + return False + try: + return float(entry.get("exp", 0)) > time.time() + except (TypeError, ValueError): + return False diff --git a/django_forms_workflows/payments/stripe_provider.py b/django_forms_workflows/payments/stripe_provider.py index 2fbee43..6f69ff5 100644 --- a/django_forms_workflows/payments/stripe_provider.py +++ b/django_forms_workflows/payments/stripe_provider.py @@ -9,19 +9,27 @@ STRIPE_SECRET_KEY = "sk_test_..." STRIPE_PUBLISHABLE_KEY = "pk_test_..." - STRIPE_WEBHOOK_SECRET = "whsec_..." # optional, for webhook verification + STRIPE_WEBHOOK_SECRET = "whsec_..." # REQUIRED to accept webhooks + +Webhooks fail closed: when ``STRIPE_WEBHOOK_SECRET`` is absent or the +signature does not verify, the event is rejected and no state is applied. """ from __future__ import annotations -import json import logging from decimal import Decimal from typing import Any from django.conf import settings -from .base import PaymentFlow, PaymentProvider, PaymentResult, PaymentStatus +from .base import ( + PaymentFlow, + PaymentProvider, + PaymentResult, + PaymentStatus, + WebhookVerificationError, +) logger = logging.getLogger(__name__) @@ -59,12 +67,20 @@ def create_payment( submission_id: int, description: str = "", metadata: dict[str, Any] | None = None, + idempotency_key: str | None = None, ) -> PaymentResult: stripe = _get_stripe() stripe.api_key = settings.STRIPE_SECRET_KEY amount_cents = int(amount * 100) + # Forward the stable per-attempt idempotency key so a retried + # initiation of the same PaymentRecord cannot create duplicate + # PaymentIntents on Stripe's side. + request_options: dict[str, Any] = {} + if idempotency_key: + request_options["idempotency_key"] = idempotency_key + try: intent = stripe.PaymentIntent.create( amount=amount_cents, @@ -72,6 +88,7 @@ def create_payment( description=description or f"Form submission #{submission_id}", metadata=metadata or {}, automatic_payment_methods={"enabled": True}, + **request_options, ) return PaymentResult( success=True, @@ -131,19 +148,27 @@ def handle_webhook(self, request: Any) -> PaymentResult | None: stripe = _get_stripe() webhook_secret = getattr(settings, "STRIPE_WEBHOOK_SECRET", "") + if not webhook_secret: + # Fail closed: without the webhook secret we cannot prove the + # event came from Stripe, so no state change may be applied. + logger.error( + "Stripe webhook received but STRIPE_WEBHOOK_SECRET is not " + "configured; refusing to process the event." + ) + raise WebhookVerificationError( + "STRIPE_WEBHOOK_SECRET is not configured; cannot verify event." + ) + payload = request.body sig_header = request.META.get("HTTP_STRIPE_SIGNATURE", "") - if webhook_secret: - try: - event = stripe.Webhook.construct_event( - payload, sig_header, webhook_secret - ) - except (ValueError, stripe.error.SignatureVerificationError) as e: - logger.warning("Stripe webhook verification failed: %s", e) - return None - else: - event = stripe.Event.construct_from(json.loads(payload), stripe.api_key) + try: + event = stripe.Webhook.construct_event(payload, sig_header, webhook_secret) + except (ValueError, stripe.error.SignatureVerificationError) as e: + logger.warning("Stripe webhook signature verification failed: %s", e) + raise WebhookVerificationError( + "Stripe webhook signature verification failed." + ) from e if event.type == "payment_intent.succeeded": intent = event.data.object diff --git a/django_forms_workflows/payments/views.py b/django_forms_workflows/payments/views.py index 48da5e9..ae61dc1 100644 --- a/django_forms_workflows/payments/views.py +++ b/django_forms_workflows/payments/views.py @@ -1,5 +1,16 @@ """ Payment views: initiate payment, handle return callbacks, process webhooks. + +Every endpoint that touches a submission or payment record enforces the +single access policy in :mod:`django_forms_workflows.payments.checkout`: + +* an authenticated submitter may act only on their own submission; +* an anonymous public-form submitter needs a short-lived, unguessable, + session-bound checkout credential. + +Callers that fail the policy get a non-disclosing 404 — identical to a +nonexistent object — and no payment details, client secrets, provider +calls, or record changes are performed. """ from __future__ import annotations @@ -10,7 +21,9 @@ import uuid from decimal import Decimal, InvalidOperation -from django.http import HttpResponse, JsonResponse +from django.contrib import messages +from django.db import transaction +from django.http import Http404, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils import timezone @@ -18,21 +31,78 @@ from django.views.decorators.http import require_POST from ..models import AuditLog, FormSubmission, PaymentRecord -from .base import PaymentFlow, PaymentStatus +from .base import PaymentFlow, PaymentStatus, WebhookVerificationError +from .checkout import has_checkout_access, revoke_checkout_access from .registry import get_provider logger = logging.getLogger(__name__) +# Payment-record states in which an attempt is still open and may be +# confirmed, returned to, retried against the provider, or cancelled. +ACTIVE_PAYMENT_STATUSES = ("pending", "requires_action", "processing") + +# Payment-record states that must never be left by a later event. +# `completed -> refunded` is the only permitted forward transition. +_LOCKED_PAYMENT_STATUSES = ("completed", "refunded") + + +def _require_checkout_access(request, submission): + """Enforce the payment access policy or raise a non-disclosing 404.""" + if not has_checkout_access(request, submission): + raise Http404("Not found") + + +def _apply_payment_result(payment_record, result) -> bool: + """ + Apply a provider result to ``payment_record`` without rolling state + backward. + + Returns True if the record changed. Events arriving for a record in a + locked state (``completed``/``refunded``) are ignored — except the + forward ``completed -> refunded`` transition — so replayed or + out-of-order events cannot revert a finished payment. + """ + new_status = result.status.value + if payment_record.status in _LOCKED_PAYMENT_STATUSES and not ( + payment_record.status == "completed" and new_status == "refunded" + ): + logger.info( + "Ignoring out-of-order payment event: record %s is %r, event says %r", + payment_record.pk, + payment_record.status, + new_status, + ) + return False + + payment_record.status = new_status + payment_record.provider_data = result.provider_data + if new_status == PaymentStatus.COMPLETED.value: + payment_record.completed_at = timezone.now() + payment_record.error_message = "" + else: + payment_record.error_message = result.error_message or "" + payment_record.save() + return True + def initiate_payment(request, submission_id): """ - Called after form validation. Creates a PaymentRecord and either: + Called after form validation. Creates or resumes a PaymentRecord and + either: - Returns the inline payment page for INLINE providers (Stripe) - Redirects to the external portal for REDIRECT providers (ECSI, PayPal) + + Initiation is idempotent per attempt: repeated navigation reuses the + existing open PaymentRecord (and its stable idempotency key) instead of + creating new provider-side payment intents. A legitimate retry — after + the previous attempt was cancelled or failed — gets a fresh record and + idempotency key. """ submission = get_object_or_404( FormSubmission, id=submission_id, status="pending_payment" ) + _require_checkout_access(request, submission) + form_def = submission.form_definition if not form_def.payment_enabled or not form_def.payment_provider: @@ -65,39 +135,59 @@ def initiate_payment(request, submission_id): currency = form_def.payment_currency or "usd" description = _resolve_description(form_def, submission) - idempotency_key = f"sub_{submission.id}_{uuid.uuid4().hex[:12]}" - - payment_record = PaymentRecord.objects.create( - submission=submission, - form_definition=form_def, - provider_name=form_def.payment_provider, - amount=amount, - currency=currency, - description=description, - status="pending", - idempotency_key=idempotency_key, - ) - result = provider.create_payment( - amount=amount, - currency=currency, - submission_id=submission.id, - description=description, - metadata={ - "form_slug": form_def.slug, - "submission_id": str(submission.id), - "payment_record_id": str(payment_record.id), - }, - ) + with transaction.atomic(): + # Lock the submission row so concurrent/repeated requests cannot + # create duplicate payment attempts. + FormSubmission.objects.select_for_update().get(pk=submission.pk) - payment_record.transaction_id = result.transaction_id - payment_record.provider_data = result.provider_data - if result.success: - payment_record.status = result.status.value - else: - payment_record.status = "failed" - payment_record.error_message = result.error_message - payment_record.save() + payment_record = ( + submission.payment_records.filter(status__in=ACTIVE_PAYMENT_STATUSES) + .order_by("-created_at") + .first() + ) + if payment_record is None: + payment_record = PaymentRecord.objects.create( + submission=submission, + form_definition=form_def, + provider_name=form_def.payment_provider, + amount=amount, + currency=currency, + description=description, + status="pending", + idempotency_key=f"sub_{submission.id}_{uuid.uuid4().hex[:12]}", + ) + + # (Re)issue the provider call with the attempt's stable idempotency + # key. Providers that support idempotency dedupe this to the + # original intent/session instead of creating a duplicate charge. + result = provider.create_payment( + amount=payment_record.amount, + currency=payment_record.currency, + submission_id=submission.id, + description=payment_record.description, + metadata={ + "form_slug": form_def.slug, + "submission_id": str(submission.id), + "payment_record_id": str(payment_record.id), + }, + idempotency_key=payment_record.idempotency_key, + ) + + payment_record.transaction_id = ( + result.transaction_id or payment_record.transaction_id + ) + payment_record.provider_data = result.provider_data + if result.success: + # Never let a (re)issued initiation leave a locked state. + if payment_record.status not in _LOCKED_PAYMENT_STATUSES: + payment_record.status = result.status.value + elif payment_record.status not in _LOCKED_PAYMENT_STATUSES: + # Likewise, a transient re-issue failure must not stomp an + # attempt that was already completed by a concurrent webhook. + payment_record.status = "failed" + payment_record.error_message = result.error_message + payment_record.save() if not result.success: return render( @@ -123,8 +213,8 @@ def initiate_payment(request, submission_id): "payment_record": payment_record, "provider_name": form_def.payment_provider, "client_config_json": json.dumps(client_config), - "amount": amount, - "currency": currency, + "amount": payment_record.amount, + "currency": payment_record.currency, }, ) @@ -137,6 +227,7 @@ def confirm_payment(request, payment_record_id): """ payment_record = get_object_or_404(PaymentRecord, id=payment_record_id) submission = payment_record.submission + _require_checkout_access(request, submission) provider = get_provider(payment_record.provider_name) if not provider: @@ -144,13 +235,8 @@ def confirm_payment(request, payment_record_id): {"success": False, "error": "Provider not found"}, status=500 ) - result = provider.confirm_payment(transaction_id=payment_record.transaction_id) - - payment_record.status = result.status.value - payment_record.provider_data = result.provider_data - if result.status == PaymentStatus.COMPLETED: - payment_record.completed_at = timezone.now() - payment_record.save() + # Idempotent replay: this attempt already finished. + if payment_record.status == PaymentStatus.COMPLETED.value: _finalize_submission(submission) return JsonResponse( { @@ -158,15 +244,35 @@ def confirm_payment(request, payment_record_id): "redirect_url": _get_success_redirect(submission), } ) - else: - payment_record.error_message = result.error_message - payment_record.save() + + # Only an open attempt on a submission still awaiting payment may be + # confirmed; anything else is a lifecycle violation. + if ( + payment_record.status not in ACTIVE_PAYMENT_STATUSES + or submission.status != "pending_payment" + ): + return JsonResponse( + {"success": False, "error": "This payment can no longer be confirmed."}, + status=409, + ) + + result = provider.confirm_payment(transaction_id=payment_record.transaction_id) + + _apply_payment_result(payment_record, result) + if payment_record.status == PaymentStatus.COMPLETED.value: + _finalize_submission(submission) return JsonResponse( { - "success": False, - "error": result.error_message or "Payment could not be confirmed.", + "success": True, + "redirect_url": _get_success_redirect(submission), } ) + return JsonResponse( + { + "success": False, + "error": result.error_message or "Payment could not be confirmed.", + } + ) def payment_return(request, submission_id): @@ -175,10 +281,17 @@ def payment_return(request, submission_id): Provider-specific query params are forwarded to confirm_payment(). """ submission = get_object_or_404(FormSubmission, id=submission_id) + _require_checkout_access(request, submission) + + # The checkout was already finalized (e.g. by a webhook racing this + # browser return) — send the user on without changing anything. + if submission.status != "pending_payment": + return redirect(_get_success_redirect(submission)) + + # Bind the return to the originating checkout: the one open attempt + # for this submission, verified server-side with the provider. payment_record = ( - submission.payment_records.filter( - status__in=["pending", "processing", "requires_action"] - ) + submission.payment_records.filter(status__in=ACTIVE_PAYMENT_STATUSES) .order_by("-created_at") .first() ) @@ -209,48 +322,52 @@ def payment_return(request, submission_id): **request.GET.dict(), ) - payment_record.status = result.status.value - payment_record.provider_data = result.provider_data - if result.status == PaymentStatus.COMPLETED: - payment_record.completed_at = timezone.now() - payment_record.save() + _apply_payment_result(payment_record, result) + if payment_record.status == PaymentStatus.COMPLETED.value: _finalize_submission(submission) return redirect(_get_success_redirect(submission)) - else: - payment_record.error_message = result.error_message - payment_record.save() - return render( - request, - "django_forms_workflows/payment_error.html", - { - "error": result.error_message or "Payment was not completed.", - "submission": submission, - }, - ) + return render( + request, + "django_forms_workflows/payment_error.html", + { + "error": result.error_message or "Payment was not completed.", + "submission": submission, + }, + ) +@require_POST def payment_cancel(request, submission_id): - """User cancelled payment on external portal.""" - from django.contrib import messages - - submission = get_object_or_404(FormSubmission, id=submission_id) - payment_record = ( - submission.payment_records.filter(status="pending") - .order_by("-created_at") - .first() + """ + User cancelled payment. CSRF-protected POST only; a plain GET (e.g. an + unauthenticated provider redirect) can no longer change state, and a + verified provider callback arrives via the webhook endpoint instead. + """ + submission = get_object_or_404( + FormSubmission, id=submission_id, status="pending_payment" ) + _require_checkout_access(request, submission) - if payment_record: + # Cancel only open attempts — never a completed or refunded payment. + for payment_record in submission.payment_records.filter( + status__in=ACTIVE_PAYMENT_STATUSES + ): payment_record.status = "cancelled" - payment_record.save() + payment_record.save(update_fields=["status", "updated_at"]) + # The status guard above scopes this to pending_payment submissions, + # so a completed/submitted/approved submission can never be reverted. submission.status = "draft" - submission.save() + submission.save(update_fields=["status"]) + + revoke_checkout_access(request, submission) messages.warning( request, "Payment was cancelled. Your form has been saved as a draft." ) - return redirect("forms_workflows:my_submissions") + if request.user.is_authenticated: + return redirect("forms_workflows:my_submissions") + return redirect("forms_workflows:form_list") @csrf_exempt @@ -259,7 +376,11 @@ def payment_webhook(request, provider_name): """ Generic webhook endpoint: /forms/payments/webhook// - Delegates to the provider's handle_webhook() method. + Delegates to the provider's handle_webhook() method. Events are + applied only when the provider authenticates them (e.g. a verified + Stripe signature); unverifiable events fail closed with HTTP 400. + Replayed or out-of-order events never roll a payment's state backward + or create duplicate workflow tasks. """ provider = get_provider(provider_name) if not provider: @@ -267,6 +388,11 @@ def payment_webhook(request, provider_name): try: result = provider.handle_webhook(request) + except WebhookVerificationError: + logger.warning( + "Rejected unauthenticated webhook event for provider %s", provider_name + ) + return HttpResponse(status=400) except Exception: logger.exception("Webhook processing error for provider %s", provider_name) return HttpResponse(status=500) @@ -284,12 +410,9 @@ def payment_webhook(request, provider_name): logger.warning("Webhook for unknown transaction %s", result.transaction_id) return HttpResponse(status=200) - payment_record.status = result.status.value - payment_record.provider_data = result.provider_data - if result.status == PaymentStatus.COMPLETED: - payment_record.completed_at = timezone.now() + _apply_payment_result(payment_record, result) + if payment_record.status == PaymentStatus.COMPLETED.value: _finalize_submission(payment_record.submission) - payment_record.save() return HttpResponse(status=200) @@ -341,6 +464,9 @@ def _finalize_submission(submission): comments="Payment completed, form submitted", ) + # A stale checkout credential cannot be reused after finalization: + # every endpoint requires the submission to still be pending_payment. + from ..views import create_approval_tasks create_approval_tasks(submission) diff --git a/django_forms_workflows/templates/django_forms_workflows/payment_collect.html b/django_forms_workflows/templates/django_forms_workflows/payment_collect.html index 8a47a5e..f85092f 100644 --- a/django_forms_workflows/templates/django_forms_workflows/payment_collect.html +++ b/django_forms_workflows/templates/django_forms_workflows/payment_collect.html @@ -35,8 +35,14 @@
Complete Payment
- Cancel payment +
+ {% csrf_token %} + +
diff --git a/django_forms_workflows/templates/django_forms_workflows/payment_error.html b/django_forms_workflows/templates/django_forms_workflows/payment_error.html index 723775b..101f49e 100644 --- a/django_forms_workflows/templates/django_forms_workflows/payment_error.html +++ b/django_forms_workflows/templates/django_forms_workflows/payment_error.html @@ -17,9 +17,12 @@

Payment Error

Try Again - - Cancel Payment - +
+ {% csrf_token %} + +
{% else %} Back to Forms diff --git a/django_forms_workflows/views.py b/django_forms_workflows/views.py index dc666cc..fdc5371 100644 --- a/django_forms_workflows/views.py +++ b/django_forms_workflows/views.py @@ -505,6 +505,11 @@ def form_submit(request, slug): if form_def.payment_enabled and form_def.payment_provider: submission.status = "pending_payment" submission.save() + # Bind a short-lived checkout credential to this session so + # only this browser can complete the payment for it. + from .payments.checkout import grant_checkout_access + + grant_checkout_access(request, submission) AuditLog.objects.create( action="submit", object_type="FormSubmission", @@ -705,6 +710,11 @@ def form_embed(request, slug): if form_def.payment_enabled and form_def.payment_provider: submission.status = "pending_payment" submission.save() + # Bind a short-lived checkout credential to this session so + # only this browser can complete the payment for it. + from .payments.checkout import grant_checkout_access + + grant_checkout_access(request, submission) return redirect( reverse( "forms_workflows:payment_initiate", diff --git a/docs/PAYMENTS.md b/docs/PAYMENTS.md index ac1bd44..5bcbfff 100644 --- a/docs/PAYMENTS.md +++ b/docs/PAYMENTS.md @@ -89,16 +89,55 @@ A new `pending_payment` status is added to the submission lifecycle: ``` draft → pending_payment → submitted → pending_approval → approved - ↘ cancelled (if user cancels payment) + ↘ draft (only via an authorized cancellation) ``` +## Payment access policy + +Payment endpoints (`initiate`, `confirm`, `return`, `cancel`) enforce a +single access policy before rendering payment details, exposing client +secrets, calling providers, or changing records: + +- **Authenticated submissions** — only the authenticated submitter may act + on their own submission. +- **Anonymous (public-form) submissions** — the submitter's browser + receives an unguessable, **session-bound checkout credential** when the + form post creates the payment checkout. The credential is valid for + `FORMS_WORKFLOWS_PAYMENT_CHECKOUT_TTL` seconds (default **3600**). + +A numeric submission or payment-record ID alone grants **no** access; +callers that fail the policy receive a non-disclosing `404`, identical to +a nonexistent object. The credential is revoked on cancellation and cannot +be used once the submission has left `pending_payment`. + +## Initiation idempotency and retries + +Initiation is **idempotent per payment attempt**: + +- The first authorized request creates one `PaymentRecord` with a stable + `idempotency_key`; the row is locked while the attempt is created, so + concurrent or repeated requests cannot create duplicate attempts. +- Repeated navigation to the initiate URL **reuses the open attempt** + (`pending` / `requires_action` / `processing`) and re-issues the + provider call with the **same idempotency key**, so providers that + support request idempotency (e.g. Stripe) dedupe instead of creating + duplicate provider-side charges. +- A **legitimate retry** starts a new attempt with a fresh idempotency + key: cancel the open attempt (`POST …/payments//cancel/`) + or let a failed attempt be retried — the next initiation creates the + new `PaymentRecord` then. + +Cancellation is a CSRF-protected `POST` only. It is restricted to +`pending_payment` submissions and cancels only open attempts — it can +never revert a completed, submitted, or approved submission to `draft`. + ## Submission flow 1. User fills out the form and submits. -2. If `payment_enabled`, the submission is saved with `status="pending_payment"` and the user is redirected to the payment initiation endpoint. -3. The view resolves the payment amount (fixed or from a form field), creates a `PaymentRecord`, and calls the provider's `create_payment()`. +2. If `payment_enabled`, the submission is saved with `status="pending_payment"`, a checkout credential is bound to the user's session, and the user is redirected to the payment initiation endpoint. +3. The view resolves the payment amount (fixed or from a form field), creates (or resumes) a `PaymentRecord`, and calls the provider's `create_payment()` with the attempt's stable idempotency key. 4. **INLINE flow**: renders `payment_collect.html` with the provider's JS SDK (e.g. Stripe Elements). The user completes payment without leaving the site. -5. **REDIRECT flow**: redirects the user to the provider's hosted payment page. +5. **REDIRECT flow**: redirects the user to the provider's hosted payment page. The return URL is bound to the originating (single open) attempt and the provider's result is verified server-side. 6. On successful payment confirmation, the submission transitions to `submitted` and enters the normal workflow engine (`_finalize_submission()`). ## Built-in Stripe provider @@ -126,6 +165,12 @@ The Stripe provider uses **PaymentIntents** with `automatic_payment_methods` ena ### Webhook configuration +Webhooks **require signing**. Add the webhook signing secret to Django settings: + +```python +STRIPE_WEBHOOK_SECRET = "whsec_..." # REQUIRED — events are rejected without it +``` + Set up a Stripe webhook in your Stripe Dashboard pointing to: ``` @@ -134,6 +179,26 @@ https://your-domain.com/forms/payments/webhook/stripe/ Subscribe to `payment_intent.succeeded` and `payment_intent.payment_failed` events. +The provider verifies the `Stripe-Signature` header against +`STRIPE_WEBHOOK_SECRET` before any event is applied. Verification **fails +closed**: if the secret is missing or the signature is invalid, the +endpoint answers HTTP 400 and no payment or submission state changes. + +Replayed and out-of-order events are safe: a completed payment is never +rolled backward by a late or duplicate event, `_finalize_submission()` is +idempotent (no duplicate workflow tasks), and the only permitted +transition out of `completed` is `completed → refunded`. + +### Provider webhook requirement (all providers) + +Every provider's `handle_webhook(request)` MUST authenticate the event +(signature, shared secret, or provider-verified fetch) before returning a +`PaymentResult`, and raise +`django_forms_workflows.payments.base.WebhookVerificationError` when +verification fails or its verification secret is not configured. The +generic webhook endpoint turns that exception into HTTP 400 without +applying any state change. + ## Writing a custom provider Third-party providers self-register in their Django app's `ready()` method: @@ -145,6 +210,7 @@ from django_forms_workflows.payments.base import ( PaymentProvider, PaymentResult, PaymentStatus, + WebhookVerificationError, ) class MyProvider(PaymentProvider): @@ -158,8 +224,12 @@ class MyProvider(PaymentProvider): return bool(getattr(settings, "MY_GATEWAY_KEY", "")) def create_payment(self, amount, currency, description, metadata, idempotency_key): - # Call your gateway API to create a checkout session - session = my_gateway.create_session(amount=amount, currency=currency) + # Call your gateway API to create a checkout session. Forward the + # idempotency key if your gateway supports it, so a retried + # initiation of the same attempt cannot double-charge. + session = my_gateway.create_session( + amount=amount, currency=currency, idempotency_key=idempotency_key + ) return PaymentResult( success=True, status=PaymentStatus.PENDING, @@ -176,7 +246,10 @@ class MyProvider(PaymentProvider): ) def handle_webhook(self, request): - # Verify and process webhook + # Authenticate the event first — raise to reject unverified payloads. + if not my_gateway.verify_signature(request): + raise WebhookVerificationError("invalid webhook signature") + event = my_gateway.parse_event(request) return PaymentResult(...) def refund_payment(self, transaction_id, amount=None, reason=None): diff --git a/tests/test_payment_security.py b/tests/test_payment_security.py new file mode 100644 index 0000000..4e7d0e9 --- /dev/null +++ b/tests/test_payment_security.py @@ -0,0 +1,848 @@ +""" +Regression tests for payment endpoint authorization and state transitions +(issue #27). + +Covers the payment access policy (owner / anonymous session-bound checkout +credential), idempotent initiation, lifecycle-restricted confirm/return/ +cancel, fail-closed webhook signature verification, and replay/out-of-order +webhook handling. +""" + +from types import SimpleNamespace +from unittest import mock + +import pytest +from django.test import Client +from django.urls import reverse + +from django_forms_workflows.models import ( + ApprovalTask, + FormDefinition, + FormField, + FormSubmission, + PaymentRecord, +) +from django_forms_workflows.payments.base import ( + PaymentFlow, + PaymentProvider, + PaymentResult, + PaymentStatus, + WebhookVerificationError, +) +from django_forms_workflows.payments.checkout import SESSION_KEY +from django_forms_workflows.payments.registry import ( + _instances, + _registry, + register_provider, +) + +pytestmark = pytest.mark.django_db + +PROVIDER_NAME = "testpay_security" + + +# ── Fixtures ───────────────────────────────────────────────────────── + + +class FakeProvider(PaymentProvider): + """Controllable in-memory provider used to drive the payment views.""" + + def __init__(self): + self.flow = PaymentFlow.INLINE + self.create_calls = [] + self.confirm_results = [] + self.webhook_result = None + self.webhook_error = None + self.next_create_result = None + self.tx_counter = 0 + + # PaymentProvider interface ─────────────────────────────────────── + def get_name(self): + return "Fake" + + def get_flow_type(self): + return self.flow + + def is_available(self): + return True + + def create_payment( + self, + amount, + currency, + submission_id, + description="", + metadata=None, + idempotency_key=None, + ): + self.create_calls.append( + { + "amount": amount, + "currency": currency, + "submission_id": submission_id, + "idempotency_key": idempotency_key, + } + ) + if self.next_create_result is not None: + result = self.next_create_result + self.next_create_result = None + return result + self.tx_counter += 1 + return PaymentResult( + success=True, + status=PaymentStatus.PENDING, + transaction_id=f"txn_{self.tx_counter}", + client_secret=f"cs_{self.tx_counter}", + provider_data={"intent_id": f"txn_{self.tx_counter}"}, + ) + + def confirm_payment(self, transaction_id, **kwargs): + if self.confirm_results: + return self.confirm_results.pop(0) + return PaymentResult( + success=True, + status=PaymentStatus.COMPLETED, + transaction_id=transaction_id, + provider_data={"status": "succeeded"}, + ) + + def handle_webhook(self, request): + if self.webhook_error is not None: + raise self.webhook_error + return self.webhook_result + + def get_client_config(self, payment_result): + return {"provider": "fake", "client_secret": payment_result.client_secret} + + +@pytest.fixture +def client(): + return Client() + + +@pytest.fixture +def auth_client(client, user): + client.force_login(user) + return client + + +@pytest.fixture +def fake_provider(): + provider = FakeProvider() + register_provider(PROVIDER_NAME, provider) + try: + yield provider + finally: + _registry.pop(PROVIDER_NAME, None) + _instances.pop(PROVIDER_NAME, None) + + +@pytest.fixture +def paid_form(db, category, user): + return FormDefinition.objects.create( + name="Paid Security Form", + slug="paid-security-form", + category=category, + is_active=True, + payment_enabled=True, + payment_provider=PROVIDER_NAME, + payment_amount_type="fixed", + payment_fixed_amount="25.00", + payment_currency="usd", + created_by=user, + ) + + +@pytest.fixture +def paid_public_form(db, category, user): + form_def = FormDefinition.objects.create( + name="Paid Public Form", + slug="paid-public-form", + category=category, + is_active=True, + requires_login=False, + payment_enabled=True, + payment_provider=PROVIDER_NAME, + payment_amount_type="fixed", + payment_fixed_amount="25.00", + payment_currency="usd", + created_by=user, + ) + FormField.objects.create( + form_definition=form_def, + field_name="full_name", + field_label="Full Name", + field_type="text", + order=1, + required=True, + ) + return form_def + + +@pytest.fixture +def pending_submission(paid_form, user): + return FormSubmission.objects.create( + form_definition=paid_form, + submitter=user, + form_data={"full_name": "Owner"}, + status="pending_payment", + ) + + +@pytest.fixture +def pending_record(paid_form, pending_submission): + return PaymentRecord.objects.create( + submission=pending_submission, + form_definition=paid_form, + provider_name=PROVIDER_NAME, + amount="25.00", + currency="usd", + status="pending", + transaction_id="txn_pending_1", + idempotency_key="sub_sec_key_1", + ) + + +@pytest.fixture +def other_user(db): + from django.contrib.auth.models import User + + return User.objects.create_user( + username="other", email="other@example.com", password="pass123" + ) + + +@pytest.fixture +def other_client(other_user): + client = Client() + client.force_login(other_user) + return client + + +@pytest.fixture +def anonymous_checkout(paid_public_form, fake_provider): + """Anonymous browser that has just posted a public paid form. + + Returns (client, submission): the client session holds a valid + checkout credential for the submission. + """ + client = Client() + resp = client.post( + reverse("forms_workflows:form_submit", args=[paid_public_form.slug]), + data={"full_name": "Anonymous Payer"}, + ) + assert resp.status_code == 302 + assert resp["Location"].endswith("/initiate/") + submission = FormSubmission.objects.get( + form_definition=paid_public_form, status="pending_payment" + ) + return client, submission + + +def _initiate_url(submission): + return reverse("forms_workflows:payment_initiate", args=[submission.id]) + + +def _confirm_url(record): + return reverse("forms_workflows:payment_confirm", args=[record.id]) + + +def _return_url(submission): + return reverse("forms_workflows:payment_return", args=[submission.id]) + + +def _cancel_url(submission): + return reverse("forms_workflows:payment_cancel", args=[submission.id]) + + +# ── Access policy: authenticated owner / strangers / anonymous ─────── + + +class TestPaymentAccessPolicy: + def test_anonymous_gets_404_on_owned_submission( + self, client, pending_submission, fake_provider + ): + resp = client.get(_initiate_url(pending_submission)) + assert resp.status_code == 404 + assert fake_provider.create_calls == [] + + def test_different_signed_in_user_gets_404( + self, other_client, pending_submission, fake_provider + ): + resp = other_client.get(_initiate_url(pending_submission)) + assert resp.status_code == 404 + assert fake_provider.create_calls == [] + + def test_owner_can_initiate(self, auth_client, pending_submission, fake_provider): + resp = auth_client.get(_initiate_url(pending_submission)) + assert resp.status_code == 200 + assert len(fake_provider.create_calls) == 1 + + def test_confirm_denied_for_anonymous(self, client, pending_record, fake_provider): + resp = client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 404 + + def test_confirm_denied_for_different_user( + self, other_client, pending_record, fake_provider + ): + resp = other_client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 404 + + def test_return_denied_for_anonymous( + self, client, pending_submission, pending_record, fake_provider + ): + resp = client.get(_return_url(pending_submission)) + assert resp.status_code == 404 + + def test_return_denied_for_different_user( + self, other_client, pending_submission, pending_record, fake_provider + ): + resp = other_client.get(_return_url(pending_submission)) + assert resp.status_code == 404 + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + + def test_cancel_denied_for_anonymous( + self, client, pending_submission, fake_provider + ): + resp = client.post(_cancel_url(pending_submission)) + assert resp.status_code == 404 + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + + def test_cancel_denied_for_different_user( + self, other_client, pending_submission, fake_provider + ): + resp = other_client.post(_cancel_url(pending_submission)) + assert resp.status_code == 404 + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + + +# ── Anonymous public-form checkout credential ──────────────────────── + + +class TestAnonymousCheckoutCredential: + def test_form_post_grants_session_credential(self, anonymous_checkout): + client, submission = anonymous_checkout + session = client.session + assert str(submission.id) in session.get(SESSION_KEY, {}) + + def test_anonymous_with_credential_can_initiate(self, anonymous_checkout): + client, submission = anonymous_checkout + resp = client.get(_initiate_url(submission)) + assert resp.status_code == 200 + + def test_guessed_id_without_credential(self, paid_public_form, fake_provider): + stranger = Client() + submission = FormSubmission.objects.create( + form_definition=paid_public_form, + submitter=None, + form_data={"full_name": "Victim"}, + status="pending_payment", + ) + for url in ( + _initiate_url(submission), + _return_url(submission), + ): + assert stranger.get(url).status_code == 404 + assert stranger.post(_cancel_url(submission)).status_code == 404 + assert PaymentRecord.objects.count() == 0 + submission.refresh_from_db() + assert submission.status == "pending_payment" + + def test_credential_does_not_transfer_across_submissions( + self, paid_public_form, anonymous_checkout + ): + client, own_submission = anonymous_checkout + other_submission = FormSubmission.objects.create( + form_definition=paid_public_form, + submitter=None, + form_data={"full_name": "Someone Else"}, + status="pending_payment", + ) + resp = client.get(_initiate_url(other_submission)) + assert resp.status_code == 404 + resp = client.post(_cancel_url(other_submission)) + assert resp.status_code == 404 + + def test_expired_credential_is_rejected(self, anonymous_checkout, settings): + client, submission = anonymous_checkout + # Forge an already-expired credential. + session = client.session + session[SESSION_KEY][str(submission.id)]["exp"] = 1 + session.save() + resp = client.get(_initiate_url(submission)) + assert resp.status_code == 404 + + def test_anonymous_checkout_full_payment_cycle( + self, anonymous_checkout, fake_provider + ): + client, submission = anonymous_checkout + resp = client.get(_initiate_url(submission)) + assert resp.status_code == 200 + record = PaymentRecord.objects.get(submission=submission) + + resp = client.post(_confirm_url(record), {}) + assert resp.status_code == 200 + assert resp.json()["success"] is True + submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert submission.status == "approved" + + +# ── GET vs POST ────────────────────────────────────────────────────── + + +class TestHttpMethods: + def test_cancel_rejects_get(self, auth_client, pending_submission): + resp = auth_client.get(_cancel_url(pending_submission)) + assert resp.status_code == 405 + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + + def test_confirm_rejects_get(self, auth_client, pending_record): + resp = auth_client.get(_confirm_url(pending_record)) + assert resp.status_code == 405 + + def test_webhook_rejects_get(self, client): + resp = client.get(reverse("forms_workflows:payment_webhook", args=["stripe"])) + assert resp.status_code == 405 + + +# ── Idempotent initiation / repeated requests ──────────────────────── + + +class TestIdempotentInitiation: + def test_repeated_gets_reuse_single_attempt( + self, auth_client, pending_submission, fake_provider + ): + for _ in range(3): + resp = auth_client.get(_initiate_url(pending_submission)) + assert resp.status_code == 200 + + records = PaymentRecord.objects.filter(submission=pending_submission) + assert records.count() == 1 + record = records.first() + # The provider was re-invoked with the SAME stable idempotency key, + # so an idempotency-aware provider dedupes instead of re-charging. + keys = [c["idempotency_key"] for c in fake_provider.create_calls] + assert keys == [record.idempotency_key] * 3 + + def test_retry_after_cancel_starts_new_attempt( + self, auth_client, pending_submission, fake_provider + ): + resp = auth_client.get(_initiate_url(pending_submission)) + assert resp.status_code == 200 + PaymentRecord.objects.get(submission=pending_submission) + + resp = auth_client.post(_cancel_url(pending_submission)) + assert resp.status_code == 302 + pending_submission.refresh_from_db() + assert pending_submission.status == "draft" + + # A legitimate retry: move the submission back to pending_payment + # (as a resubmission of the draft would) and re-initiate. + pending_submission.status = "pending_payment" + pending_submission.save(update_fields=["status"]) + resp = auth_client.get(_initiate_url(pending_submission)) + assert resp.status_code == 200 + + records = PaymentRecord.objects.filter(submission=pending_submission).order_by( + "created_at" + ) + assert records.count() == 2 + assert records[0].status == "cancelled" + assert records[1].status == "pending" + assert records[0].idempotency_key != records[1].idempotency_key + + +# ── Confirm: lifecycle restrictions ───────────────────────────────── + + +class TestConfirmLifecycle: + def test_confirm_completes_and_finalizes( + self, auth_client, pending_record, fake_provider + ): + resp = auth_client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 200 + assert resp.json()["success"] is True + pending_record.refresh_from_db() + assert pending_record.status == "completed" + assert pending_record.completed_at is not None + pending_record.submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert pending_record.submission.status == "approved" + + def test_confirm_is_idempotent_replay( + self, auth_client, pending_record, fake_provider + ): + for _ in range(2): + resp = auth_client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 200 + assert resp.json()["success"] is True + pending_record.refresh_from_db() + assert pending_record.status == "completed" + + def test_confirm_rejects_cancelled_record( + self, auth_client, pending_record, fake_provider + ): + pending_record.status = "cancelled" + pending_record.save(update_fields=["status"]) + resp = auth_client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 409 + assert resp.json()["success"] is False + + def test_confirm_rejects_when_submission_not_pending( + self, auth_client, pending_record, fake_provider, user + ): + submission = pending_record.submission + submission.status = "submitted" + submission.save(update_fields=["status"]) + pending_record.status = "failed" + pending_record.save(update_fields=["status"]) + resp = auth_client.post(_confirm_url(pending_record), {}) + assert resp.status_code == 409 + + +# ── Return: provider redirect binding ─────────────────────────────── + + +class TestPaymentReturn: + def test_return_completes_checkout( + self, auth_client, pending_submission, pending_record, fake_provider + ): + resp = auth_client.get(_return_url(pending_submission)) + assert resp.status_code == 302 + pending_record.refresh_from_db() + assert pending_record.status == "completed" + pending_submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert pending_submission.status == "approved" + + def test_return_after_finalization_is_safe( + self, auth_client, pending_submission, pending_record, fake_provider + ): + # Simulate a webhook finalizing before the browser returns. + pending_submission.status = "submitted" + pending_submission.save(update_fields=["status"]) + pending_record.status = "completed" + pending_record.save(update_fields=["status"]) + + resp = auth_client.get(_return_url(pending_submission)) + assert resp.status_code == 302 + pending_submission.refresh_from_db() + assert pending_submission.status == "submitted" + + def test_return_failed_payment_does_not_finalize( + self, auth_client, pending_submission, pending_record, fake_provider + ): + fake_provider.confirm_results.append( + PaymentResult( + success=False, + status=PaymentStatus.FAILED, + transaction_id="txn_pending_1", + error_message="card declined", + ) + ) + resp = auth_client.get(_return_url(pending_submission)) + assert resp.status_code == 200 # error page + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + pending_record.refresh_from_db() + assert pending_record.status == "failed" + + +# ── Cancel: state transition safety ───────────────────────────────── + + +class TestCancelTransitions: + def test_cancel_reverts_pending_payment_to_draft( + self, auth_client, pending_submission, pending_record + ): + resp = auth_client.post(_cancel_url(pending_submission)) + assert resp.status_code == 302 + pending_submission.refresh_from_db() + assert pending_submission.status == "draft" + pending_record.refresh_from_db() + assert pending_record.status == "cancelled" + + def test_cancel_never_reverts_submitted( + self, auth_client, pending_submission, user + ): + pending_submission.status = "submitted" + pending_submission.save(update_fields=["status"]) + resp = auth_client.post(_cancel_url(pending_submission)) + assert resp.status_code == 404 + pending_submission.refresh_from_db() + assert pending_submission.status == "submitted" + + @pytest.mark.parametrize("status", ["approved", "pending_approval"]) + def test_cancel_never_reverts_later_states( + self, auth_client, pending_submission, status + ): + pending_submission.status = status + pending_submission.save(update_fields=["status"]) + resp = auth_client.post(_cancel_url(pending_submission)) + assert resp.status_code == 404 + pending_submission.refresh_from_db() + assert pending_submission.status == status + + def test_cancel_does_not_touch_completed_record( + self, auth_client, pending_submission, pending_record + ): + pending_record.status = "completed" + pending_record.save(update_fields=["status"]) + auth_client.post(_cancel_url(pending_submission)) + pending_record.refresh_from_db() + assert pending_record.status == "completed" + pending_submission.refresh_from_db() + assert pending_submission.status == "draft" + + def test_anonymous_cancel_revokes_credential( + self, anonymous_checkout, fake_provider + ): + client, submission = anonymous_checkout + resp = client.post(_cancel_url(submission)) + assert resp.status_code == 302 + submission.refresh_from_db() + assert submission.status == "draft" + assert str(submission.id) not in client.session.get(SESSION_KEY, {}) + + +# ── Webhooks: signature verification required ─────────────────────── + + +def _make_payment_record(submission, transaction_id="txn_wh_1", status="pending"): + return PaymentRecord.objects.create( + submission=submission, + form_definition=submission.form_definition, + provider_name=PROVIDER_NAME, + amount="25.00", + currency="usd", + status=status, + transaction_id=transaction_id, + idempotency_key=f"wh_key_{transaction_id}_{status}", + ) + + +def _webhook_url(provider=PROVIDER_NAME): + return reverse("forms_workflows:payment_webhook", args=[provider]) + + +class TestWebhookVerification: + def test_unverifiable_event_rejected( + self, client, pending_submission, fake_provider + ): + record = _make_payment_record(pending_submission) + fake_provider.webhook_error = WebhookVerificationError("no secret") + resp = client.post(_webhook_url(), data="{}", content_type="application/json") + assert resp.status_code == 400 + record.refresh_from_db() + assert record.status == "pending" + pending_submission.refresh_from_db() + assert pending_submission.status == "pending_payment" + + def test_verified_event_applies(self, client, pending_submission, fake_provider): + record = _make_payment_record(pending_submission) + fake_provider.webhook_result = PaymentResult( + success=True, + status=PaymentStatus.COMPLETED, + transaction_id="txn_wh_1", + provider_data={"event_type": "payment_intent.succeeded"}, + ) + resp = client.post(_webhook_url(), data="{}", content_type="application/json") + assert resp.status_code == 200 + record.refresh_from_db() + assert record.status == "completed" + assert record.completed_at is not None + pending_submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert pending_submission.status == "approved" + + def test_duplicate_event_creates_no_duplicate_tasks( + self, client, paid_form, user, fake_provider, approval_group + ): + from django_forms_workflows.models import WorkflowDefinition, WorkflowStage + + wf = WorkflowDefinition.objects.create( + form_definition=paid_form, requires_approval=True + ) + stage = WorkflowStage.objects.create( + workflow=wf, name="Approval", order=1, approval_logic="any" + ) + stage.approval_groups.add(approval_group) + + submission = FormSubmission.objects.create( + form_definition=paid_form, + submitter=user, + form_data={}, + status="pending_payment", + ) + _make_payment_record(submission) + fake_provider.webhook_result = PaymentResult( + success=True, + status=PaymentStatus.COMPLETED, + transaction_id="txn_wh_1", + provider_data={"event_type": "payment_intent.succeeded"}, + ) + for _ in range(3): + resp = client.post( + _webhook_url(), data="{}", content_type="application/json" + ) + assert resp.status_code == 200 + submission.refresh_from_db() + assert submission.status == "pending_approval" + task_count = ApprovalTask.objects.filter(submission=submission).count() + for _ in range(2): # extra replays still change nothing + client.post(_webhook_url(), data="{}", content_type="application/json") + assert ApprovalTask.objects.filter(submission=submission).count() == task_count + + def test_out_of_order_failure_does_not_roll_back_completion( + self, client, pending_submission, fake_provider + ): + record = _make_payment_record(pending_submission, status="completed") + pending_submission.status = "submitted" + pending_submission.save(update_fields=["status"]) + fake_provider.webhook_result = PaymentResult( + success=False, + status=PaymentStatus.FAILED, + transaction_id="txn_wh_1", + error_message="late failure event", + ) + resp = client.post(_webhook_url(), data="{}", content_type="application/json") + assert resp.status_code == 200 + record.refresh_from_db() + assert record.status == "completed" + pending_submission.refresh_from_db() + assert pending_submission.status == "submitted" + + def test_failed_record_can_still_complete( + self, client, pending_submission, fake_provider + ): + record = _make_payment_record(pending_submission, status="failed") + fake_provider.webhook_result = PaymentResult( + success=True, + status=PaymentStatus.COMPLETED, + transaction_id="txn_wh_1", + provider_data={"event_type": "payment_intent.succeeded"}, + ) + resp = client.post(_webhook_url(), data="{}", content_type="application/json") + assert resp.status_code == 200 + record.refresh_from_db() + assert record.status == "completed" + pending_submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert pending_submission.status == "approved" + + +# ── Stripe provider: fail-closed webhook verification ─────────────── + + +class _FakeStripeError: + SignatureVerificationError = type("SignatureVerificationError", (Exception,), {}) + + +def _fake_stripe(construct_event=None): + return SimpleNamespace( + Webhook=SimpleNamespace(construct_event=construct_event), + error=_FakeStripeError, + api_key=None, + ) + + +class TestStripeWebhookVerification: + URL = reverse("forms_workflows:payment_webhook", args=["stripe"]) + + def _post(self, client): + return client.post(self.URL, data=b"{}", content_type="application/json") + + def test_missing_webhook_secret_fails_closed(self, client, settings): + settings.STRIPE_WEBHOOK_SECRET = "" + with mock.patch( + "django_forms_workflows.payments.stripe_provider._get_stripe", + return_value=_fake_stripe(), + ): + resp = self._post(client) + assert resp.status_code == 400 + + def test_invalid_signature_fails_closed(self, client, settings): + settings.STRIPE_WEBHOOK_SECRET = "whsec_test" + + def _construct(payload, sig, secret): + raise _FakeStripeError.SignatureVerificationError("bad signature") + + with mock.patch( + "django_forms_workflows.payments.stripe_provider._get_stripe", + return_value=_fake_stripe(construct_event=_construct), + ): + resp = self._post(client) + assert resp.status_code == 400 + + def test_valid_signature_applies_event(self, client, settings, paid_form, user): + settings.STRIPE_WEBHOOK_SECRET = "whsec_test" + settings.STRIPE_SECRET_KEY = "sk_test_x" + submission = FormSubmission.objects.create( + form_definition=paid_form, + submitter=user, + form_data={}, + status="pending_payment", + ) + record = PaymentRecord.objects.create( + submission=submission, + form_definition=paid_form, + provider_name="stripe", + amount="25.00", + currency="usd", + status="pending", + transaction_id="pi_123", + idempotency_key="stripe_test_key", + ) + + event = SimpleNamespace( + type="payment_intent.succeeded", + data=SimpleNamespace(object=SimpleNamespace(id="pi_123")), + ) + fake = _fake_stripe(construct_event=lambda p, s, k: event) + with mock.patch( + "django_forms_workflows.payments.stripe_provider._get_stripe", + return_value=fake, + ): + resp = self._post(client) + assert resp.status_code == 200 + record.refresh_from_db() + assert record.status == "completed" + submission.refresh_from_db() + # No approval workflow on this form — the engine auto-approves. + assert submission.status == "approved" + + def test_stripe_forwards_idempotency_key(self, settings): + settings.STRIPE_SECRET_KEY = "sk_test_x" + captured = {} + + class FakeIntent: + id = "pi_test" + client_secret = "cs_test" + status = "requires_payment_method" + + def _create(**kwargs): + captured.update(kwargs) + return FakeIntent() + + fake = SimpleNamespace( + PaymentIntent=SimpleNamespace(create=_create), + api_key=None, + ) + from django_forms_workflows.payments.stripe_provider import ( + StripePaymentProvider, + ) + + with mock.patch( + "django_forms_workflows.payments.stripe_provider._get_stripe", + return_value=fake, + ): + result = StripePaymentProvider().create_payment( + amount=10, currency="usd", submission_id=1, idempotency_key="k_1" + ) + assert captured.get("idempotency_key") == "k_1" + assert result.success is True