diff --git a/backend/internal/payment/provider/wxpay.go b/backend/internal/payment/provider/wxpay.go index b06f352720..b53e566fa9 100644 --- a/backend/internal/payment/provider/wxpay.go +++ b/backend/internal/payment/provider/wxpay.go @@ -78,11 +78,14 @@ func NewWxpay(instanceID string, config map[string]string) (*Wxpay, error) { "actual": strconv.Itoa(len(config["apiV3Key"])), }) } - // publicKey + publicKeyId are a pair used by the new pubkey verifier. - // If either is set, both must be set; otherwise fall back to legacy platform certificate mode. - if (config["publicKey"] != "") != (config["publicKeyId"] != "") { - return nil, infraerrors.BadRequest("WXPAY_CONFIG_PAIR_VIOLATION", "pair_violation"). - WithMetadata(map[string]string{"keys": "publicKey/publicKeyId"}) + // Pubkey verifier mode is opt-in via publicKeyId. If publicKeyId is set, + // publicKey must also be set so the verifier can load it. + // A leftover publicKey without publicKeyId is treated as unused legacy data, + // not an error, so admins can switch back to platform-certificate mode without + // having to manually clear the old publicKey field. + if config["publicKeyId"] != "" && config["publicKey"] == "" { + return nil, infraerrors.BadRequest("WXPAY_CONFIG_MISSING_KEY", "missing_required_key"). + WithMetadata(map[string]string{"key": "publicKey"}) } return &Wxpay{instanceID: instanceID, config: config}, nil } diff --git a/backend/internal/payment/provider/wxpay_test.go b/backend/internal/payment/provider/wxpay_test.go index 111cc16932..960988b9ef 100644 --- a/backend/internal/payment/provider/wxpay_test.go +++ b/backend/internal/payment/provider/wxpay_test.go @@ -218,16 +218,16 @@ func TestNewWxpay(t *testing.T) { wantErr: false, }, { - name: "publicKey without publicKeyId", - config: withOverride(map[string]string{"publicKeyId": ""}), - wantErr: true, - errSubstr: "WXPAY_CONFIG_PAIR_VIOLATION", + // Leftover publicKey from a former pubkey-mode setup is ignored; treated as legacy mode. + name: "publicKey leftover without publicKeyId is allowed", + config: withOverride(map[string]string{"publicKeyId": ""}), + wantErr: false, }, { name: "publicKeyId without publicKey", config: withOverride(map[string]string{"publicKey": ""}), wantErr: true, - errSubstr: "WXPAY_CONFIG_PAIR_VIOLATION", + errSubstr: "WXPAY_CONFIG_MISSING_KEY", }, { name: "apiV3Key too short", diff --git a/backend/internal/service/payment_config_providers.go b/backend/internal/service/payment_config_providers.go index c6c86adf36..30ff4253f5 100644 --- 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) { @@ -137,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 @@ -210,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 }