fix(payment): critical audit fixes for security, idempotency and correctness

Backend fixes:
- #1: doSub subscription idempotency via audit log check
- #2: markFailed only when status=RECHARGING (prevents overwriting COMPLETED)
- #3: ExpireTimedOutOrders checks upstream payment before expiring
- #4: Public verify endpoint for payment result page (no auth required)
- #5: EasyPay QueryOrder returns amount, confirmPayment handles zero amount
- #6: WxPay notifyUrl priority: request-first, config-fallback
- #7: EasyPay remove double URL decode in VerifyNotification
- #8: checkPaid/cancelUpstreamPayment use order's provider instance
- #9: Amount NaN/Inf/negative validation in order creation and refund
- #10: Refund amount comparison uses tolerance instead of float64 ==
- #11: Skip balance deduction on retry when previous rollback failed
- #12: checkPaid logs fulfillment errors instead of silently ignoring
- #13: WxPay certSerial added to required config fields

Frontend fixes:
- Payment result page no longer requires authentication
- Public verify API fallback for expired sessions
This commit is contained in:
erio
2026-04-10 02:23:19 +08:00
parent 72b77306da
commit db139191e3
11 changed files with 183 additions and 81 deletions
@@ -363,6 +363,41 @@ func (h *PaymentHandler) VerifyOrder(c *gin.Context) {
response.Success(c, order)
}
// PublicOrderResult is the limited order info returned by the public verify endpoint.
// No user details are exposed — only payment status information.
type PublicOrderResult struct {
ID int64 `json:"id"`
OutTradeNo string `json:"out_trade_no"`
Amount float64 `json:"amount"`
PayAmount float64 `json:"pay_amount"`
PaymentType string `json:"payment_type"`
Status string `json:"status"`
}
// VerifyOrderPublic verifies payment status without requiring authentication.
// Returns limited order info (no user details) to prevent information leakage.
// POST /api/v1/payment/public/orders/verify
func (h *PaymentHandler) VerifyOrderPublic(c *gin.Context) {
var req VerifyOrderRequest
if err := c.ShouldBindJSON(&req); err != nil {
response.BadRequest(c, "Invalid request: "+err.Error())
return
}
order, err := h.paymentService.VerifyOrderPublic(c.Request.Context(), req.OutTradeNo)
if err != nil {
response.ErrorFrom(c, err)
return
}
response.Success(c, PublicOrderResult{
ID: order.ID,
OutTradeNo: order.OutTradeNo,
Amount: order.Amount,
PayAmount: order.PayAmount,
PaymentType: order.PaymentType,
Status: order.Status,
})
}
// requireAuth extracts the authenticated subject from the context.
// Returns the subject and true on success; on failure it writes an Unauthorized response and returns false.
func requireAuth(c *gin.Context) (middleware2.AuthSubject, bool) {
+5 -2
View File
@@ -158,6 +158,7 @@ func (e *EasyPay) QueryOrder(ctx context.Context, tradeNo string) (*payment.Quer
Code int `json:"code"`
Msg string `json:"msg"`
Status int `json:"status"`
Money string `json:"money"`
}
if err := json.Unmarshal(body, &resp); err != nil {
return nil, fmt.Errorf("easypay parse query: %w", err)
@@ -166,7 +167,8 @@ func (e *EasyPay) QueryOrder(ctx context.Context, tradeNo string) (*payment.Quer
if resp.Status == easypayStatusPaid {
status = payment.ProviderStatusPaid
}
return &payment.QueryOrderResponse{TradeNo: tradeNo, Status: status}, nil
amount, _ := strconv.ParseFloat(resp.Money, 64)
return &payment.QueryOrderResponse{TradeNo: tradeNo, Status: status, Amount: amount}, nil
}
func (e *EasyPay) VerifyNotification(_ context.Context, rawBody string, _ map[string]string) (*payment.PaymentNotification, error) {
@@ -174,9 +176,10 @@ func (e *EasyPay) VerifyNotification(_ context.Context, rawBody string, _ map[st
if err != nil {
return nil, fmt.Errorf("parse notify: %w", err)
}
// url.ParseQuery already decodes values — no additional decode needed.
params := make(map[string]string)
for k := range values {
params[k] = decodeURLValue(values.Get(k))
params[k] = values.Get(k)
}
sign := params["sign"]
if sign == "" {
+4 -3
View File
@@ -57,7 +57,7 @@ type Wxpay struct {
}
func NewWxpay(instanceID string, config map[string]string) (*Wxpay, error) {
required := []string{"appId", "mchId", "privateKey", "apiV3Key", "publicKey", "publicKeyId"}
required := []string{"appId", "mchId", "privateKey", "apiV3Key", "publicKey", "publicKeyId", "certSerial"}
for _, k := range required {
if config[k] == "" {
return nil, fmt.Errorf("wxpay config missing required key: %s", k)
@@ -127,9 +127,10 @@ func (w *Wxpay) CreatePayment(ctx context.Context, req payment.CreatePaymentRequ
if err != nil {
return nil, err
}
notifyURL := w.config["notifyUrl"]
// Request-first, config-fallback (consistent with EasyPay/Alipay)
notifyURL := req.NotifyURL
if notifyURL == "" {
notifyURL = req.NotifyURL
notifyURL = w.config["notifyUrl"]
}
if notifyURL == "" {
return nil, fmt.Errorf("wxpay notifyUrl is required")
@@ -156,6 +156,7 @@ func TestNewWxpay(t *testing.T) {
"apiV3Key": "12345678901234567890123456789012", // exactly 32 bytes
"publicKey": "fake-public-key",
"publicKeyId": "key-id-001",
"certSerial": "SERIAL001",
}
// helper to clone and override config fields
@@ -42,6 +42,14 @@ func RegisterPaymentRoutes(
}
}
// --- Public payment endpoints (no auth) ---
// Payment result page needs to verify order status without login
// (user session may have expired during provider redirect).
public := v1.Group("/payment/public")
{
public.POST("/orders/verify", paymentHandler.VerifyOrderPublic)
}
// --- Webhook endpoints (no auth) ---
webhook := v1.Group("/payment/webhook")
{
+37 -13
View File
@@ -10,6 +10,7 @@ import (
"time"
dbent "github.com/Wei-Shaw/sub2api/ent"
"github.com/Wei-Shaw/sub2api/ent/paymentauditlog"
"github.com/Wei-Shaw/sub2api/ent/paymentorder"
"github.com/Wei-Shaw/sub2api/internal/payment"
infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors"
@@ -40,9 +41,17 @@ func (s *PaymentService) confirmPayment(ctx context.Context, oid int64, tradeNo
slog.Error("order not found", "orderID", oid)
return nil
}
if math.Abs(paid-o.PayAmount) > amountToleranceCNY {
s.writeAuditLog(ctx, o.ID, "PAYMENT_AMOUNT_MISMATCH", pk, map[string]any{"expected": o.PayAmount, "paid": paid, "tradeNo": tradeNo})
return fmt.Errorf("amount mismatch: expected %.2f, got %.2f", o.PayAmount, paid)
// Skip amount check when paid=0 (e.g. QueryOrder doesn't return amount).
// Also skip if paid is NaN/Inf (malformed provider data).
if paid > 0 && !math.IsNaN(paid) && !math.IsInf(paid, 0) {
if math.Abs(paid-o.PayAmount) > amountToleranceCNY {
s.writeAuditLog(ctx, o.ID, "PAYMENT_AMOUNT_MISMATCH", pk, map[string]any{"expected": o.PayAmount, "paid": paid, "tradeNo": tradeNo})
return fmt.Errorf("amount mismatch: expected %.2f, got %.2f", o.PayAmount, paid)
}
}
// Use order's expected amount when provider didn't report one
if paid <= 0 || math.IsNaN(paid) || math.IsInf(paid, 0) {
paid = o.PayAmount
}
return s.toPaid(ctx, o, tradeNo, paid, pk)
}
@@ -249,27 +258,42 @@ func (s *PaymentService) doSub(ctx context.Context, o *dbent.PaymentOrder) error
if err != nil || g.Status != payment.EntityStatusActive {
return fmt.Errorf("group %d no longer exists or inactive", gid)
}
_, _, err = s.subscriptionSvc.AssignOrExtendSubscription(ctx, &AssignSubscriptionInput{UserID: o.UserID, GroupID: gid, ValidityDays: days, AssignedBy: 0, Notes: fmt.Sprintf("payment order %d", o.ID)})
// Idempotency: check audit log to see if subscription was already assigned.
// Prevents double-extension on retry after markCompleted fails.
if s.hasAuditLog(ctx, o.ID, "SUBSCRIPTION_SUCCESS") {
slog.Info("subscription already assigned for order, skipping", "orderID", o.ID, "groupID", gid)
return s.markCompleted(ctx, o, "SUBSCRIPTION_SUCCESS")
}
orderNote := fmt.Sprintf("payment order %d", o.ID)
_, _, err = s.subscriptionSvc.AssignOrExtendSubscription(ctx, &AssignSubscriptionInput{UserID: o.UserID, GroupID: gid, ValidityDays: days, AssignedBy: 0, Notes: orderNote})
if err != nil {
return fmt.Errorf("assign subscription: %w", err)
}
now := time.Now()
_, err = s.entClient.PaymentOrder.Update().Where(paymentorder.IDEQ(o.ID), paymentorder.StatusEQ(OrderStatusRecharging)).SetStatus(OrderStatusCompleted).SetCompletedAt(now).Save(ctx)
if err != nil {
return fmt.Errorf("mark completed: %w", err)
}
s.writeAuditLog(ctx, o.ID, "SUBSCRIPTION_SUCCESS", "system", map[string]any{"groupId": gid, "days": days, "amount": o.Amount})
return nil
return s.markCompleted(ctx, o, "SUBSCRIPTION_SUCCESS")
}
func (s *PaymentService) hasAuditLog(ctx context.Context, orderID int64, action string) bool {
oid := strconv.FormatInt(orderID, 10)
c, _ := s.entClient.PaymentAuditLog.Query().
Where(paymentauditlog.OrderIDEQ(oid), paymentauditlog.ActionEQ(action)).
Limit(1).Count(ctx)
return c > 0
}
func (s *PaymentService) markFailed(ctx context.Context, oid int64, cause error) {
now := time.Now()
r := psErrMsg(cause)
_, e := s.entClient.PaymentOrder.UpdateOneID(oid).SetStatus(OrderStatusFailed).SetFailedAt(now).SetFailedReason(r).Save(ctx)
// Only mark FAILED if still in RECHARGING state — prevents overwriting
// a COMPLETED order when markCompleted failed but fulfillment succeeded.
c, e := s.entClient.PaymentOrder.Update().
Where(paymentorder.IDEQ(oid), paymentorder.StatusEQ(OrderStatusRecharging)).
SetStatus(OrderStatusFailed).SetFailedAt(now).SetFailedReason(r).Save(ctx)
if e != nil {
slog.Error("mark FAILED", "orderID", oid, "error", e)
}
s.writeAuditLog(ctx, oid, "FULFILLMENT_FAILED", "system", map[string]any{"reason": r})
if c > 0 {
s.writeAuditLog(ctx, oid, "FULFILLMENT_FAILED", "system", map[string]any{"reason": r})
}
}
func (s *PaymentService) RetryFulfillment(ctx context.Context, oid int64) error {
+61 -26
View File
@@ -72,6 +72,9 @@ func (s *PaymentService) validateOrderInput(ctx context.Context, req CreateOrder
if req.OrderType == payment.OrderTypeSubscription {
return s.validateSubOrder(ctx, req)
}
if math.IsNaN(req.Amount) || math.IsInf(req.Amount, 0) || req.Amount <= 0 {
return nil, infraerrors.BadRequest("INVALID_AMOUNT", "amount must be a positive number")
}
if (cfg.MinAmount > 0 && req.Amount < cfg.MinAmount) || (cfg.MaxAmount > 0 && req.Amount > cfg.MaxAmount) {
return nil, infraerrors.BadRequest("INVALID_AMOUNT", "amount out of range").
WithMetadata(map[string]string{"min": fmt.Sprintf("%.2f", cfg.MinAmount), "max": fmt.Sprintf("%.2f", cfg.MaxAmount)})
@@ -401,7 +404,7 @@ func (s *PaymentService) AdminCancelOrder(ctx context.Context, orderID int64) (s
}
func (s *PaymentService) cancelCore(ctx context.Context, o *dbent.PaymentOrder, fs, op, ad string) (string, error) {
if o.PaymentTradeNo != "" && o.PaymentType != "" {
if o.PaymentTradeNo != "" || o.PaymentType != "" {
if s.checkPaid(ctx, o) == "already_paid" {
return "already_paid", nil
}
@@ -411,14 +414,17 @@ func (s *PaymentService) cancelCore(ctx context.Context, o *dbent.PaymentOrder,
return "", fmt.Errorf("update order status: %w", err)
}
if c > 0 {
s.writeAuditLog(ctx, o.ID, "ORDER_CANCELLED", op, map[string]any{"detail": ad})
auditAction := "ORDER_CANCELLED"
if fs == OrderStatusExpired {
auditAction = "ORDER_EXPIRED"
}
s.writeAuditLog(ctx, o.ID, auditAction, op, map[string]any{"detail": ad})
}
return "cancelled", nil
}
func (s *PaymentService) checkPaid(ctx context.Context, o *dbent.PaymentOrder) string {
s.EnsureProviders(ctx)
prov, err := s.registry.GetProvider(o.PaymentType)
prov, err := s.getOrderProvider(ctx, o)
if err != nil {
return ""
}
@@ -434,11 +440,14 @@ func (s *PaymentService) checkPaid(ctx context.Context, o *dbent.PaymentOrder) s
return ""
}
if resp.Status == payment.ProviderStatusPaid {
_ = s.HandlePaymentNotification(ctx, &payment.PaymentNotification{TradeNo: o.PaymentTradeNo, OrderID: o.OutTradeNo, Amount: resp.Amount, Status: payment.ProviderStatusSuccess}, prov.ProviderKey())
if err := s.HandlePaymentNotification(ctx, &payment.PaymentNotification{TradeNo: o.PaymentTradeNo, OrderID: o.OutTradeNo, Amount: resp.Amount, Status: payment.ProviderStatusSuccess}, prov.ProviderKey()); err != nil {
slog.Error("fulfillment failed during checkPaid", "orderID", o.ID, "error", err)
// Still return already_paid — order was paid, fulfillment can be retried
}
return "already_paid"
}
if cp, ok := prov.(payment.CancelableProvider); ok {
_ = cp.CancelPayment(ctx, o.PaymentTradeNo)
_ = cp.CancelPayment(ctx, tradeNo)
}
return ""
}
@@ -470,6 +479,27 @@ func (s *PaymentService) VerifyOrderByOutTradeNo(ctx context.Context, outTradeNo
return o, nil
}
// VerifyOrderPublic verifies payment status without user authentication.
// Used by the payment result page when the user's session has expired.
func (s *PaymentService) VerifyOrderPublic(ctx context.Context, outTradeNo string) (*dbent.PaymentOrder, error) {
o, err := s.entClient.PaymentOrder.Query().
Where(paymentorder.OutTradeNo(outTradeNo)).
Only(ctx)
if err != nil {
return nil, infraerrors.NotFound("NOT_FOUND", "order not found")
}
if o.Status == OrderStatusPending || o.Status == OrderStatusExpired {
result := s.checkPaid(ctx, o)
if result == "already_paid" {
o, err = s.entClient.PaymentOrder.Get(ctx, o.ID)
if err != nil {
return nil, fmt.Errorf("reload order: %w", err)
}
}
}
return o, nil
}
func (s *PaymentService) ExpireTimedOutOrders(ctx context.Context) (int, error) {
now := time.Now()
orders, err := s.entClient.PaymentOrder.Query().Where(paymentorder.StatusEQ(OrderStatusPending), paymentorder.ExpiresAtLTE(now)).All(ctx)
@@ -478,34 +508,39 @@ func (s *PaymentService) ExpireTimedOutOrders(ctx context.Context) (int, error)
}
n := 0
for _, o := range orders {
// Cancel upstream payment (e.g. Stripe PaymentIntent) before marking expired
s.cancelUpstreamPayment(ctx, o)
c, e := s.entClient.PaymentOrder.Update().Where(paymentorder.IDEQ(o.ID), paymentorder.StatusEQ(OrderStatusPending)).SetStatus(OrderStatusExpired).Save(ctx)
if e != nil {
slog.Warn("expire failed", "orderID", o.ID, "error", e)
// Check upstream payment status before expiring — the user may have
// paid just before timeout and the webhook hasn't arrived yet.
outcome, _ := s.cancelCore(ctx, o, OrderStatusExpired, "system", "order expired")
if outcome == "already_paid" {
slog.Info("order was paid during expiry", "orderID", o.ID)
continue
}
if c > 0 {
s.writeAuditLog(ctx, o.ID, "ORDER_EXPIRED", "system", map[string]any{"expiresAt": o.ExpiresAt.Format(time.RFC3339)})
if outcome != "" {
n++
}
}
return n, nil
}
// cancelUpstreamPayment attempts to cancel the upstream provider payment (e.g. Stripe PaymentIntent).
func (s *PaymentService) cancelUpstreamPayment(ctx context.Context, o *dbent.PaymentOrder) {
if o.PaymentTradeNo == "" || o.PaymentType == "" {
return
}
s.EnsureProviders(ctx)
prov, err := s.registry.GetProvider(o.PaymentType)
if err != nil {
return
}
if cp, ok := prov.(payment.CancelableProvider); ok {
if err := cp.CancelPayment(ctx, o.PaymentTradeNo); err != nil {
slog.Warn("cancel upstream payment failed", "orderID", o.ID, "tradeNo", o.PaymentTradeNo, "error", err)
// getOrderProvider creates a provider using the order's original instance config.
// Falls back to registry lookup if instance ID is missing (legacy orders).
func (s *PaymentService) getOrderProvider(ctx context.Context, o *dbent.PaymentOrder) (payment.Provider, error) {
if o.ProviderInstanceID != nil && *o.ProviderInstanceID != "" {
instID, err := strconv.ParseInt(*o.ProviderInstanceID, 10, 64)
if err == nil {
cfg, err := s.loadBalancer.GetInstanceConfig(ctx, instID)
if err == nil {
providerKey := s.registry.GetProviderKey(o.PaymentType)
if providerKey == "" {
providerKey = o.PaymentType
}
p, err := provider.CreateProvider(providerKey, *o.ProviderInstanceID, cfg)
if err == nil {
return p, nil
}
}
}
}
s.EnsureProviders(ctx)
return s.registry.GetProvider(o.PaymentType)
}
+18 -31
View File
@@ -12,7 +12,6 @@ import (
dbent "github.com/Wei-Shaw/sub2api/ent"
"github.com/Wei-Shaw/sub2api/ent/paymentorder"
"github.com/Wei-Shaw/sub2api/internal/payment"
"github.com/Wei-Shaw/sub2api/internal/payment/provider"
infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors"
)
@@ -70,14 +69,18 @@ func (s *PaymentService) PrepareRefund(ctx context.Context, oid int64, amt float
if !psSliceContains(ok, o.Status) {
return nil, nil, infraerrors.BadRequest("INVALID_STATUS", "order status does not allow refund")
}
if math.IsNaN(amt) || math.IsInf(amt, 0) {
return nil, nil, infraerrors.BadRequest("INVALID_AMOUNT", "invalid refund amount")
}
if amt <= 0 {
amt = o.Amount
}
if amt > o.Amount {
if amt-o.Amount > amountToleranceCNY {
return nil, nil, infraerrors.BadRequest("REFUND_AMOUNT_EXCEEDED", "refund amount exceeds recharge")
}
// Full refund: use actual pay_amount for gateway (includes fees)
ga := amt
if amt == o.Amount {
if math.Abs(amt-o.Amount) <= amountToleranceCNY {
ga = o.PayAmount
}
rr := strings.TrimSpace(reason)
@@ -122,9 +125,16 @@ func (s *PaymentService) ExecuteRefund(ctx context.Context, p *RefundPlan) (*Ref
return nil, infraerrors.Conflict("CONFLICT", "order status changed")
}
if p.DeductionType == payment.DeductionTypeBalance && p.BalanceToDeduct > 0 {
if err := s.userRepo.DeductBalance(ctx, p.Order.UserID, p.BalanceToDeduct); err != nil {
s.restoreStatus(ctx, p)
return nil, fmt.Errorf("deduction: %w", err)
// Skip balance deduction on retry if previous attempt already deducted
// but failed to roll back (REFUND_ROLLBACK_FAILED in audit log).
if !s.hasAuditLog(ctx, p.OrderID, "REFUND_ROLLBACK_FAILED") {
if err := s.userRepo.DeductBalance(ctx, p.Order.UserID, p.BalanceToDeduct); err != nil {
s.restoreStatus(ctx, p)
return nil, fmt.Errorf("deduction: %w", err)
}
} else {
slog.Warn("skipping balance deduction on retry (previous rollback failed)", "orderID", p.OrderID)
p.BalanceToDeduct = 0
}
}
if err := s.gwRefund(ctx, p); err != nil {
@@ -155,32 +165,9 @@ func (s *PaymentService) gwRefund(ctx context.Context, p *RefundPlan) error {
}
// getRefundProvider creates a provider using the order's original instance config.
// Falls back to registry lookup if instance ID is missing (legacy orders).
// Delegates to getOrderProvider which handles instance lookup and fallback.
func (s *PaymentService) getRefundProvider(ctx context.Context, o *dbent.PaymentOrder) (payment.Provider, error) {
if o.ProviderInstanceID != nil && *o.ProviderInstanceID != "" {
instID, err := strconv.ParseInt(*o.ProviderInstanceID, 10, 64)
if err == nil {
cfg, err := s.loadBalancer.GetInstanceConfig(ctx, instID)
if err == nil {
providerKey := s.registry.GetProviderKey(o.PaymentType)
if providerKey == "" {
providerKey = o.PaymentType
}
p, err := provider.CreateProvider(providerKey, *o.ProviderInstanceID, cfg)
if err == nil {
return p, nil
}
slog.Warn("failed to create provider from instance, falling back to registry",
"instance_id", instID, "error", err)
} else {
slog.Warn("failed to get instance config for refund, falling back to registry",
"instance_id", instID, "error", err)
}
}
}
// Fallback: use registry (may pick wrong instance for multi-instance setups)
s.EnsureProviders(ctx)
return s.registry.GetProvider(o.PaymentType)
return s.getOrderProvider(ctx, o)
}
func (s *PaymentService) handleGwFail(ctx context.Context, p *RefundPlan, gErr error) (*RefundResult, error) {
+5
View File
@@ -67,6 +67,11 @@ export const paymentAPI = {
return apiClient.post<PaymentOrder>('/payment/orders/verify', { out_trade_no: outTradeNo })
},
/** Verify order payment status without auth (public endpoint for result page) */
verifyOrderPublic(outTradeNo: string) {
return apiClient.post<PaymentOrder>('/payment/public/orders/verify', { out_trade_no: outTradeNo })
},
/** Request a refund for a completed order */
requestRefund(id: number, data: { reason: string }) {
return apiClient.post(`/payment/orders/${id}/refund-request`, data)
+2 -2
View File
@@ -231,11 +231,11 @@ const routes: RouteRecordRaw[] = [
name: 'PaymentResult',
component: () => import('@/views/user/PaymentResultView.vue'),
meta: {
requiresAuth: true,
requiresAuth: false,
requiresAdmin: false,
title: 'Payment Result',
titleKey: 'payment.result.success',
requiresPayment: true
requiresPayment: false
}
},
{
@@ -136,14 +136,17 @@ onMounted(async () => {
}
}
// If we have an out_trade_no from a provider return URL, actively verify
// the payment with the upstream provider (handles missed notify callbacks)
// Verify payment via public endpoint (works without login)
if (outTradeNo) {
try {
const result = await paymentAPI.verifyOrder(outTradeNo)
const result = await paymentAPI.verifyOrderPublic(outTradeNo)
order.value = result.data
} catch (_err: unknown) {
// Verification failed, fall through to normal order lookup
// Public verify failed, try authenticated endpoint if logged in
try {
const result = await paymentAPI.verifyOrder(outTradeNo)
order.value = result.data
} catch (_e: unknown) { /* fall through */ }
}
}