feat(payment): harden wxpay config validation with structured errors

Motivation: platform-certificate mode is being phased out by WeChat (2024-10+, newly-provisioned merchants already cannot download platform certificates at all), and wxpay config errors currently surface only when an order is being created — admins have no feedback at save time. Also, errors were returned as natural-language strings, leaving the frontend no way to localize them. Changes: - backend/internal/payment/provider/wxpay.go - Replace fmt.Errorf with structured infraerrors.BadRequest errors: - WXPAY_CONFIG_MISSING_KEY (metadata: key) - WXPAY_CONFIG_INVALID_KEY_LENGTH (metadata: key, expected, actual) - WXPAY_CONFIG_INVALID_KEY (metadata: key) for malformed PEMs - Parse privateKey and publicKey PEMs in NewWxpay so malformed keys fail at save time instead of at order creation. - Keep the pubkey verifier (NewSHA256WithRSAPubkeyVerifier) as the single supported verifier; no more loadKeyPair helper. - backend/internal/service/payment_order.go invokeProvider - If CreateProvider or CreatePayment returns a structured ApplicationError, pass it through (optionally enriching metadata with provider/instance_id) instead of wrapping it as generic PAYMENT_GATEWAY_ERROR — so clients see the actual reason code (e.g. WXPAY_CONFIG_MISSING_KEY) and can localize. - Simplify a few messages (TOO_MANY_PENDING, DAILY_LIMIT_EXCEEDED, PAYMENT_GATEWAY_ERROR, NO_AVAILABLE_INSTANCE) to keyword form with metadata for template variables. - backend/internal/service/payment_config_providers.go - New helper validateProviderConfig calls provider.CreateProvider at save time. Enabled instances are validated on both Create and Update so admins see config errors immediately in the dialog, not later at order creation. - Disabled instances are not validated (half-filled drafts are allowed). - backend/internal/payment/provider/wxpay_test.go - Add generateTestKeyPair helper that produces valid RSA-2048 PKCS8/PKIX PEMs per test, used by the valid-config baseline (prior fake strings no longer pass the eager PEM check). - Cover each structured-error branch (missing/invalid-length/malformed PEM).
2026-04-20 19:49:45 +08:00
parent 23def40bc5
commit 79192cf65b
4 changed files with 159 additions and 41 deletions
--- a/backend/internal/payment/provider/wxpay.go
+++ b/backend/internal/payment/provider/wxpay.go
@@ -3,16 +3,17 @@ package provider
 import (
 	"bytes"
 	"context"
-	"crypto/rsa"
 	"fmt"
 	"io"
 	"log/slog"
 	"net/http"
+	"strconv"
 	"strings"
 	"sync"
 	"time"

 	"github.com/Wei-Shaw/sub2api/internal/payment"
+	infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors"
 	"github.com/wechatpay-apiv3/wechatpay-go/core"
 	"github.com/wechatpay-apiv3/wechatpay-go/core/auth/verifiers"
 	"github.com/wechatpay-apiv3/wechatpay-go/core/notify"
@@ -56,15 +57,35 @@ type Wxpay struct {
 	notifyHandler *notify.Handler
 }

+const wxpayAPIv3KeyLength = 32
+
 func NewWxpay(instanceID string, config map[string]string) (*Wxpay, error) {
-	required := []string{"appId", "mchId", "privateKey", "apiV3Key", "publicKey", "publicKeyId", "certSerial"}
+	// All fields are required. Platform-certificate mode is intentionally unsupported —
+	// WeChat has been migrating all merchants to the pubkey verifier since 2024-10,
+	// and newly-provisioned merchants cannot download platform certificates at all.
+	required := []string{"appId", "mchId", "privateKey", "apiV3Key", "certSerial", "publicKey", "publicKeyId"}
 	for _, k := range required {
 		if config[k] == "" {
-			return nil, fmt.Errorf("wxpay config missing required key: %s", k)
+			return nil, infraerrors.BadRequest("WXPAY_CONFIG_MISSING_KEY", "missing_required_key").
+				WithMetadata(map[string]string{"key": k})
 		}
 	}
-	if len(config["apiV3Key"]) != 32 {
-		return nil, fmt.Errorf("wxpay apiV3Key must be exactly 32 bytes, got %d", len(config["apiV3Key"]))
+	if len(config["apiV3Key"]) != wxpayAPIv3KeyLength {
+		return nil, infraerrors.BadRequest("WXPAY_CONFIG_INVALID_KEY_LENGTH", "invalid_key_length").
+			WithMetadata(map[string]string{
+				"key":      "apiV3Key",
+				"expected": strconv.Itoa(wxpayAPIv3KeyLength),
+				"actual":   strconv.Itoa(len(config["apiV3Key"])),
+			})
+	}
+	// Parse PEMs eagerly so malformed keys surface at save time, not at order creation.
+	if _, err := utils.LoadPrivateKey(formatPEM(config["privateKey"], "PRIVATE KEY")); err != nil {
+		return nil, infraerrors.BadRequest("WXPAY_CONFIG_INVALID_KEY", "invalid_key").
+			WithMetadata(map[string]string{"key": "privateKey"})
+	}
+	if _, err := utils.LoadPublicKey(formatPEM(config["publicKey"], "PUBLIC KEY")); err != nil {
+		return nil, infraerrors.BadRequest("WXPAY_CONFIG_INVALID_KEY", "invalid_key").
+			WithMetadata(map[string]string{"key": "publicKey"})
 	}
 	return &Wxpay{instanceID: instanceID, config: config}, nil
 }
@@ -89,14 +110,19 @@ func (w *Wxpay) ensureClient() (*core.Client, error) {
 	if w.coreClient != nil {
 		return w.coreClient, nil
 	}
-	privateKey, publicKey, err := w.loadKeyPair()
+	privateKey, err := utils.LoadPrivateKey(formatPEM(w.config["privateKey"], "PRIVATE KEY"))
 	if err != nil {
-		return nil, err
+		return nil, infraerrors.BadRequest("WXPAY_CONFIG_INVALID_KEY", "invalid_key").
+			WithMetadata(map[string]string{"key": "privateKey"})
+	}
+	publicKey, err := utils.LoadPublicKey(formatPEM(w.config["publicKey"], "PUBLIC KEY"))
+	if err != nil {
+		return nil, infraerrors.BadRequest("WXPAY_CONFIG_INVALID_KEY", "invalid_key").
+			WithMetadata(map[string]string{"key": "publicKey"})
 	}
-	certSerial := w.config["certSerial"]
 	verifier := verifiers.NewSHA256WithRSAPubkeyVerifier(w.config["publicKeyId"], *publicKey)
 	client, err := core.NewClient(context.Background(),
-		option.WithMerchantCredential(w.config["mchId"], certSerial, privateKey),
+		option.WithMerchantCredential(w.config["mchId"], w.config["certSerial"], privateKey),
 		option.WithVerifier(verifier))
 	if err != nil {
 		return nil, fmt.Errorf("wxpay init client: %w", err)
@@ -110,18 +136,6 @@ func (w *Wxpay) ensureClient() (*core.Client, error) {
 	return w.coreClient, nil
 }

-func (w *Wxpay) loadKeyPair() (*rsa.PrivateKey, *rsa.PublicKey, error) {
-	privateKey, err := utils.LoadPrivateKey(formatPEM(w.config["privateKey"], "PRIVATE KEY"))
-	if err != nil {
-		return nil, nil, fmt.Errorf("wxpay load private key: %w", err)
-	}
-	publicKey, err := utils.LoadPublicKey(formatPEM(w.config["publicKey"], "PUBLIC KEY"))
-	if err != nil {
-		return nil, nil, fmt.Errorf("wxpay load public key: %w", err)
-	}
-	return privateKey, publicKey, nil
-}
-
 func (w *Wxpay) CreatePayment(ctx context.Context, req payment.CreatePaymentRequest) (*payment.CreatePaymentResponse, error) {
 	client, err := w.ensureClient()
 	if err != nil {
--- a/backend/internal/payment/provider/wxpay_test.go
+++ b/backend/internal/payment/provider/wxpay_test.go
@@ -3,12 +3,36 @@
 package provider

 import (
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
 	"strings"
 	"testing"

 	"github.com/Wei-Shaw/sub2api/internal/payment"
 )

+// generateTestKeyPair returns a fresh RSA 2048 key pair as PEM strings.
+// The wechatpay-go SDK expects PKCS8 private keys and PKIX public keys.
+func generateTestKeyPair(t *testing.T) (privPEM, pubPEM string) {
+	t.Helper()
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatalf("generate rsa key: %v", err)
+	}
+	privDER, err := x509.MarshalPKCS8PrivateKey(key)
+	if err != nil {
+		t.Fatalf("marshal pkcs8: %v", err)
+	}
+	pubDER, err := x509.MarshalPKIXPublicKey(&key.PublicKey)
+	if err != nil {
+		t.Fatalf("marshal pkix: %v", err)
+	}
+	return string(pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: privDER})),
+		string(pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDER}))
+}
+
 func TestMapWxState(t *testing.T) {
 	t.Parallel()

@@ -149,13 +173,14 @@ func TestFormatPEM(t *testing.T) {
 func TestNewWxpay(t *testing.T) {
 	t.Parallel()

+	privPEM, pubPEM := generateTestKeyPair(t)
 	validConfig := map[string]string{
 		"appId":       "wx1234567890",
 		"mchId":       "1234567890",
-		"privateKey":  "fake-private-key",
+		"privateKey":  privPEM,
 		"apiV3Key":    "12345678901234567890123456789012", // exactly 32 bytes
-		"publicKey":   "fake-public-key",
-		"publicKeyId": "key-id-001",
+		"publicKey":   pubPEM,
+		"publicKeyId": "PUB_KEY_ID_TEST",
 		"certSerial":  "SERIAL001",
 	}

@@ -206,6 +231,12 @@ func TestNewWxpay(t *testing.T) {
 			wantErr:   true,
 			errSubstr: "apiV3Key",
 		},
+		{
+			name:      "missing certSerial",
+			config:    withOverride(map[string]string{"certSerial": ""}),
+			wantErr:   true,
+			errSubstr: "certSerial",
+		},
 		{
 			name:      "missing publicKey",
 			config:    withOverride(map[string]string{"publicKey": ""}),
@@ -218,17 +249,29 @@ func TestNewWxpay(t *testing.T) {
 			wantErr:   true,
 			errSubstr: "publicKeyId",
 		},
+		{
+			name:      "malformed privateKey PEM",
+			config:    withOverride(map[string]string{"privateKey": "not-a-valid-pem"}),
+			wantErr:   true,
+			errSubstr: "WXPAY_CONFIG_INVALID_KEY",
+		},
+		{
+			name:      "malformed publicKey PEM",
+			config:    withOverride(map[string]string{"publicKey": "not-a-valid-pem"}),
+			wantErr:   true,
+			errSubstr: "WXPAY_CONFIG_INVALID_KEY",
+		},
 		{
 			name:      "apiV3Key too short",
 			config:    withOverride(map[string]string{"apiV3Key": "short"}),
 			wantErr:   true,
-			errSubstr: "exactly 32 bytes",
+			errSubstr: "WXPAY_CONFIG_INVALID_KEY_LENGTH",
 		},
 		{
 			name:      "apiV3Key too long",
 			config:    withOverride(map[string]string{"apiV3Key": "123456789012345678901234567890123"}), // 33 bytes
 			wantErr:   true,
-			errSubstr: "exactly 32 bytes",
+			errSubstr: "WXPAY_CONFIG_INVALID_KEY_LENGTH",
 		},
 	}

--- a/backend/internal/service/payment_config_providers.go
+++ b/backend/internal/service/payment_config_providers.go
@@ -12,9 +12,22 @@ import (
 	"github.com/Wei-Shaw/sub2api/ent/paymentorder"
 	"github.com/Wei-Shaw/sub2api/ent/paymentproviderinstance"
 	"github.com/Wei-Shaw/sub2api/internal/payment"
+	"github.com/Wei-Shaw/sub2api/internal/payment/provider"
 	infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors"
 )

+// validateProviderConfig runs the provider's constructor to surface config-level
+// errors at save time (e.g. wxpay missing certSerial), instead of only failing
+// when an order is created. Returns the structured ApplicationError from the
+// constructor so the frontend i18n layer can localize it.
+//
+// Only validates enabled instances — a disabled instance may be a half-filled
+// draft the admin will complete later.
+func (s *PaymentConfigService) validateProviderConfig(providerKey string, config map[string]string) error {
+	_, err := provider.CreateProvider(providerKey, "_validate_", config)
+	return err
+}
+
 // --- Provider Instance CRUD ---

 func (s *PaymentConfigService) ListProviderInstances(ctx context.Context) ([]*dbent.PaymentProviderInstance, error) {
@@ -48,9 +61,8 @@ func (s *PaymentConfigService) ListProviderInstancesWithConfig(ctx context.Conte
 		resp := ProviderInstanceResponse{
 			ID: int64(inst.ID), ProviderKey: inst.ProviderKey, Name: inst.Name,
 			SupportedTypes: splitTypes(inst.SupportedTypes), Limits: inst.Limits,
-			Enabled: inst.Enabled, RefundEnabled: inst.RefundEnabled,
-			AllowUserRefund: inst.AllowUserRefund,
-			SortOrder:       inst.SortOrder, PaymentMode: inst.PaymentMode,
+			Enabled: inst.Enabled, RefundEnabled: inst.RefundEnabled, AllowUserRefund: inst.AllowUserRefund,
+			SortOrder: inst.SortOrder, PaymentMode: inst.PaymentMode,
 		}
 		resp.Config, err = s.decryptAndMaskConfig(inst.ProviderKey, inst.Config)
 		if err != nil {
@@ -138,6 +150,11 @@ func (s *PaymentConfigService) CreateProviderInstance(ctx context.Context, req C
 	if err := validateProviderRequest(req.ProviderKey, req.Name, typesStr); err != nil {
 		return nil, err
 	}
+	if req.Enabled {
+		if err := s.validateProviderConfig(req.ProviderKey, req.Config); err != nil {
+			return nil, err
+		}
+	}
 	enc, err := s.encryptConfig(req.Config)
 	if err != nil {
 		return nil, err
@@ -211,16 +228,42 @@ func (s *PaymentConfigService) UpdateProviderInstance(ctx context.Context, id in
 				WithMetadata(map[string]string{"count": strconv.Itoa(count)})
 		}
 	}
+	// Validate merged config when the instance will end up enabled.
+	// This surfaces provider-level errors (e.g. wxpay missing certSerial) at save time,
+	// so admins see them in the dialog instead of only when an order is created.
+	inst, err := s.entClient.PaymentProviderInstance.Get(ctx, id)
+	if err != nil {
+		return nil, fmt.Errorf("load provider instance: %w", err)
+	}
+	finalEnabled := inst.Enabled
+	if req.Enabled != nil {
+		finalEnabled = *req.Enabled
+	}
+	var mergedConfig map[string]string
+	if req.Config != nil {
+		mergedConfig, err = s.mergeConfig(ctx, id, req.Config)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if finalEnabled {
+		configToValidate := mergedConfig
+		if configToValidate == nil {
+			configToValidate, err = s.decryptConfig(inst.Config)
+			if err != nil {
+				return nil, fmt.Errorf("decrypt existing config: %w", err)
+			}
+		}
+		if err := s.validateProviderConfig(inst.ProviderKey, configToValidate); err != nil {
+			return nil, err
+		}
+	}
 	u := s.entClient.PaymentProviderInstance.UpdateOneID(id)
 	if req.Name != nil {
 		u.SetName(*req.Name)
 	}
-	if req.Config != nil {
-		merged, err := s.mergeConfig(ctx, id, req.Config)
-		if err != nil {
-			return nil, err
-		}
-		enc, err := s.encryptConfig(merged)
+	if mergedConfig != nil {
+		enc, err := s.encryptConfig(mergedConfig)
 		if err != nil {
 			return nil, err
 		}
--- a/backend/internal/service/payment_order.go
+++ b/backend/internal/service/payment_order.go
@@ -2,6 +2,7 @@ package service

 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	"math"
@@ -167,7 +168,7 @@ func (s *PaymentService) checkPendingLimit(ctx context.Context, tx *dbent.Tx, us
 		return fmt.Errorf("count pending orders: %w", err)
 	}
 	if c >= max {
-		return infraerrors.TooManyRequests("TOO_MANY_PENDING", fmt.Sprintf("too many pending orders (max %d)", max)).
+		return infraerrors.TooManyRequests("TOO_MANY_PENDING", "too_many_pending").
 			WithMetadata(map[string]string{"max": strconv.Itoa(max)})
 	}
 	return nil
@@ -191,7 +192,8 @@ func (s *PaymentService) checkDailyLimit(ctx context.Context, tx *dbent.Tx, user
 		used += o.Amount
 	}
 	if used+amount > limit {
-		return infraerrors.TooManyRequests("DAILY_LIMIT_EXCEEDED", fmt.Sprintf("daily recharge limit reached, remaining: %.2f", math.Max(0, limit-used)))
+		return infraerrors.TooManyRequests("DAILY_LIMIT_EXCEEDED", "daily_limit_exceeded").
+			WithMetadata(map[string]string{"remaining": fmt.Sprintf("%.2f", math.Max(0, limit-used))})
 	}
 	return nil
 }
@@ -201,21 +203,37 @@ func (s *PaymentService) invokeProvider(ctx context.Context, order *dbent.Paymen
 	// This enables cross-provider load balancing (e.g. EasyPay + Alipay direct for "alipay").
 	sel, err := s.loadBalancer.SelectInstance(ctx, "", req.PaymentType, payment.Strategy(cfg.LoadBalanceStrategy), payAmount)
 	if err != nil {
-		return nil, infraerrors.ServiceUnavailable("PAYMENT_GATEWAY_ERROR", fmt.Sprintf("payment method (%s) is not configured", req.PaymentType))
+		return nil, infraerrors.ServiceUnavailable("PAYMENT_GATEWAY_ERROR", "method_not_configured").
+			WithMetadata(map[string]string{"payment_type": req.PaymentType})
 	}
 	if sel == nil {
-		return nil, infraerrors.TooManyRequests("NO_AVAILABLE_INSTANCE", "no available payment instance")
+		return nil, infraerrors.TooManyRequests("NO_AVAILABLE_INSTANCE", "no_available_instance")
 	}
 	prov, err := provider.CreateProvider(sel.ProviderKey, sel.InstanceID, sel.Config)
 	if err != nil {
-		return nil, infraerrors.ServiceUnavailable("PAYMENT_GATEWAY_ERROR", "payment method is temporarily unavailable")
+		slog.Error("[PaymentService] CreateProvider failed", "provider", sel.ProviderKey, "instance", sel.InstanceID, "error", err)
+		// If the provider returned a structured ApplicationError (e.g. WXPAY_CONFIG_MISSING_KEY),
+		// pass it through with provider context added to metadata. Otherwise wrap as PAYMENT_PROVIDER_MISCONFIGURED.
+		if appErr := new(infraerrors.ApplicationError); errors.As(err, &appErr) {
+			md := map[string]string{"provider": sel.ProviderKey, "instance_id": sel.InstanceID}
+			for k, v := range appErr.Metadata {
+				md[k] = v
+			}
+			return nil, appErr.WithMetadata(md)
+		}
+		return nil, infraerrors.ServiceUnavailable("PAYMENT_PROVIDER_MISCONFIGURED", "provider_misconfigured").
+			WithMetadata(map[string]string{"provider": sel.ProviderKey, "instance_id": sel.InstanceID})
 	}
 	subject := s.buildPaymentSubject(plan, limitAmount, cfg)
 	outTradeNo := order.OutTradeNo
 	pr, err := prov.CreatePayment(ctx, payment.CreatePaymentRequest{OrderID: outTradeNo, Amount: payAmountStr, PaymentType: req.PaymentType, Subject: subject, ClientIP: req.ClientIP, IsMobile: req.IsMobile, InstanceSubMethods: sel.SupportedTypes})
 	if err != nil {
 		slog.Error("[PaymentService] CreatePayment failed", "provider", sel.ProviderKey, "instance", sel.InstanceID, "error", err)
-		return nil, infraerrors.ServiceUnavailable("PAYMENT_GATEWAY_ERROR", fmt.Sprintf("payment gateway error: %s", err.Error()))
+		if appErr := new(infraerrors.ApplicationError); errors.As(err, &appErr) {
+			return nil, appErr
+		}
+		return nil, infraerrors.ServiceUnavailable("PAYMENT_GATEWAY_ERROR", "payment_gateway_error").
+			WithMetadata(map[string]string{"provider": sel.ProviderKey, "instance_id": sel.InstanceID})
 	}
 	_, err = s.entClient.PaymentOrder.UpdateOneID(order.ID).SetNillablePaymentTradeNo(psNilIfEmpty(pr.TradeNo)).SetNillablePayURL(psNilIfEmpty(pr.PayURL)).SetNillableQrCode(psNilIfEmpty(pr.QRCode)).SetNillableProviderInstanceID(psNilIfEmpty(sel.InstanceID)).Save(ctx)
 	if err != nil {