From 9742796ee727e678fabf1e4028640a764cbac895 Mon Sep 17 00:00:00 2001 From: IanShaw027 Date: Tue, 21 Apr 2026 11:41:02 +0800 Subject: [PATCH] fix: retire public payment verify and backfill trade no --- backend/internal/handler/payment_handler.go | 34 +-- .../handler/payment_handler_resume_test.go | 53 ++++ backend/internal/server/routes/payment.go | 5 +- .../service/payment_order_lifecycle.go | 14 +- .../service/payment_order_lifecycle_test.go | 251 ++++++++++++++++++ frontend/src/api/__tests__/payment.spec.ts | 36 +++ frontend/src/api/payment.ts | 5 - .../user/__tests__/PaymentResultView.spec.ts | 8 - 8 files changed, 369 insertions(+), 37 deletions(-) create mode 100644 backend/internal/service/payment_order_lifecycle_test.go create mode 100644 frontend/src/api/__tests__/payment.spec.ts diff --git a/backend/internal/handler/payment_handler.go b/backend/internal/handler/payment_handler.go index 5fd6b43e..273aea73 100644 --- a/backend/internal/handler/payment_handler.go +++ b/backend/internal/handler/payment_handler.go @@ -2,6 +2,7 @@ package handler import ( "fmt" + "net/http" "strconv" "strings" @@ -459,29 +460,20 @@ type PublicOrderResult struct { Status string `json:"status"` } -// VerifyOrderPublic verifies payment status without requiring authentication. -// Returns limited order info (no user details) to prevent information leakage. +var errPaymentPublicOrderVerifyRemoved = infraerrors.New( + http.StatusGone, + "PAYMENT_PUBLIC_ORDER_VERIFY_REMOVED", + "public payment order verification by out_trade_no has been removed; use resume_token recovery instead", +).WithMetadata(map[string]string{ + "replacement_endpoint": "/api/v1/payment/public/orders/resolve", + "replacement_field": "resume_token", +}) + +// VerifyOrderPublic is kept as a compatibility shim for the removed anonymous +// out_trade_no lookup endpoint and always returns HTTP 410 Gone. // 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, - OrderType: order.OrderType, - Status: order.Status, - }) + response.ErrorFrom(c, errPaymentPublicOrderVerifyRemoved) } // ResolveOrderPublicByResumeToken resolves a payment order from a signed resume token. diff --git a/backend/internal/handler/payment_handler_resume_test.go b/backend/internal/handler/payment_handler_resume_test.go index 323f7292..28da15d9 100644 --- a/backend/internal/handler/payment_handler_resume_test.go +++ b/backend/internal/handler/payment_handler_resume_test.go @@ -3,10 +3,24 @@ package handler import ( + "bytes" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" "testing" + dbent "github.com/Wei-Shaw/sub2api/ent" + "github.com/Wei-Shaw/sub2api/ent/enttest" "github.com/Wei-Shaw/sub2api/internal/payment" + "github.com/Wei-Shaw/sub2api/internal/pkg/response" "github.com/Wei-Shaw/sub2api/internal/service" + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "entgo.io/ent/dialect" + entsql "entgo.io/ent/dialect/sql" + _ "modernc.org/sqlite" ) func TestApplyWeChatPaymentResumeClaims(t *testing.T) { @@ -59,3 +73,42 @@ func TestApplyWeChatPaymentResumeClaimsRejectsPaymentTypeMismatch(t *testing.T) t.Fatal("applyWeChatPaymentResumeClaims should reject mismatched payment types") } } + +func TestVerifyOrderPublicReturnsGone(t *testing.T) { + t.Parallel() + + gin.SetMode(gin.TestMode) + + db, err := sql.Open("sqlite", "file:payment_handler_public_verify?mode=memory&cache=shared") + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + _, err = db.Exec("PRAGMA foreign_keys = ON") + require.NoError(t, err) + + drv := entsql.OpenDB(dialect.SQLite, db) + client := enttest.NewClient(t, enttest.WithOptions(dbent.Driver(drv))) + t.Cleanup(func() { _ = client.Close() }) + + paymentSvc := service.NewPaymentService(client, payment.NewRegistry(), nil, nil, nil, nil, nil, nil) + h := NewPaymentHandler(paymentSvc, nil, nil) + + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest( + http.MethodPost, + "/api/v1/payment/public/orders/verify", + bytes.NewBufferString(`{"out_trade_no":"legacy-order-no"}`), + ) + ctx.Request.Header.Set("Content-Type", "application/json") + + h.VerifyOrderPublic(ctx) + + require.Equal(t, http.StatusGone, recorder.Code) + + var resp response.Response + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &resp)) + require.Equal(t, http.StatusGone, resp.Code) + require.Equal(t, "PAYMENT_PUBLIC_ORDER_VERIFY_REMOVED", resp.Reason) + require.Contains(t, resp.Message, "removed") +} diff --git a/backend/internal/server/routes/payment.go b/backend/internal/server/routes/payment.go index dff14a70..ec340d94 100644 --- a/backend/internal/server/routes/payment.go +++ b/backend/internal/server/routes/payment.go @@ -44,8 +44,9 @@ 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). + // Signed resume-token recovery is the supported public lookup path. + // The legacy anonymous out_trade_no verify endpoint is kept only as a + // compatibility shim that returns HTTP 410 Gone. public := v1.Group("/payment/public") { public.POST("/orders/verify", paymentHandler.VerifyOrderPublic) diff --git a/backend/internal/service/payment_order_lifecycle.go b/backend/internal/service/payment_order_lifecycle.go index 50a56ad0..1564c36d 100644 --- a/backend/internal/service/payment_order_lifecycle.go +++ b/backend/internal/service/payment_order_lifecycle.go @@ -151,7 +151,19 @@ func (s *PaymentService) checkPaid(ctx context.Context, o *dbent.PaymentOrder) s return "" } if resp.Status == payment.ProviderStatusPaid { - if err := s.HandlePaymentNotification(ctx, &payment.PaymentNotification{TradeNo: o.PaymentTradeNo, OrderID: o.OutTradeNo, Amount: resp.Amount, Status: payment.ProviderStatusSuccess}, prov.ProviderKey()); err != nil { + notificationTradeNo := o.PaymentTradeNo + if upstreamTradeNo := resp.TradeNo; upstreamTradeNo != "" && upstreamTradeNo != notificationTradeNo { + if _, updateErr := s.entClient.PaymentOrder.Update(). + Where(paymentorder.IDEQ(o.ID)). + SetPaymentTradeNo(upstreamTradeNo). + Save(ctx); updateErr != nil { + slog.Error("persist upstream trade no during checkPaid failed", "orderID", o.ID, "tradeNo", upstreamTradeNo, "error", updateErr) + } else { + o.PaymentTradeNo = upstreamTradeNo + } + notificationTradeNo = upstreamTradeNo + } + if err := s.HandlePaymentNotification(ctx, &payment.PaymentNotification{TradeNo: notificationTradeNo, 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 } diff --git a/backend/internal/service/payment_order_lifecycle_test.go b/backend/internal/service/payment_order_lifecycle_test.go new file mode 100644 index 00000000..3d4773a4 --- /dev/null +++ b/backend/internal/service/payment_order_lifecycle_test.go @@ -0,0 +1,251 @@ +//go:build unit + +package service + +import ( + "context" + "database/sql" + "testing" + "time" + + dbent "github.com/Wei-Shaw/sub2api/ent" + "github.com/Wei-Shaw/sub2api/ent/enttest" + "github.com/Wei-Shaw/sub2api/internal/payment" + "github.com/Wei-Shaw/sub2api/internal/pkg/pagination" + "github.com/stretchr/testify/require" + + "entgo.io/ent/dialect" + entsql "entgo.io/ent/dialect/sql" + _ "modernc.org/sqlite" +) + +type paymentOrderLifecycleQueryProvider struct { + lastQueryTradeNo string + resp *payment.QueryOrderResponse +} + +type paymentOrderLifecycleRedeemRepo struct { + codesByCode map[string]*RedeemCode + useCalls []struct { + id int64 + userID int64 + } +} + +func (p *paymentOrderLifecycleQueryProvider) Name() string { + return "payment-order-lifecycle-query-provider" +} + +func (p *paymentOrderLifecycleQueryProvider) ProviderKey() string { return payment.TypeAlipay } + +func (p *paymentOrderLifecycleQueryProvider) SupportedTypes() []payment.PaymentType { + return []payment.PaymentType{payment.TypeAlipay} +} + +func (p *paymentOrderLifecycleQueryProvider) CreatePayment(context.Context, payment.CreatePaymentRequest) (*payment.CreatePaymentResponse, error) { + panic("unexpected call") +} + +func (p *paymentOrderLifecycleQueryProvider) QueryOrder(_ context.Context, tradeNo string) (*payment.QueryOrderResponse, error) { + p.lastQueryTradeNo = tradeNo + return p.resp, nil +} + +func (p *paymentOrderLifecycleQueryProvider) VerifyNotification(context.Context, string, map[string]string) (*payment.PaymentNotification, error) { + panic("unexpected call") +} + +func (p *paymentOrderLifecycleQueryProvider) Refund(context.Context, payment.RefundRequest) (*payment.RefundResponse, error) { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) Create(context.Context, *RedeemCode) error { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) CreateBatch(context.Context, []RedeemCode) error { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) GetByID(_ context.Context, id int64) (*RedeemCode, error) { + for _, code := range r.codesByCode { + if code.ID != id { + continue + } + cloned := *code + return &cloned, nil + } + return nil, ErrRedeemCodeNotFound +} + +func (r *paymentOrderLifecycleRedeemRepo) GetByCode(_ context.Context, code string) (*RedeemCode, error) { + redeemCode, ok := r.codesByCode[code] + if !ok { + return nil, ErrRedeemCodeNotFound + } + cloned := *redeemCode + return &cloned, nil +} + +func (r *paymentOrderLifecycleRedeemRepo) Update(context.Context, *RedeemCode) error { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) Delete(context.Context, int64) error { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) Use(_ context.Context, id, userID int64) error { + for code, redeemCode := range r.codesByCode { + if redeemCode.ID != id { + continue + } + now := time.Now().UTC() + redeemCode.Status = StatusUsed + redeemCode.UsedBy = &userID + redeemCode.UsedAt = &now + r.codesByCode[code] = redeemCode + r.useCalls = append(r.useCalls, struct { + id int64 + userID int64 + }{id: id, userID: userID}) + return nil + } + return ErrRedeemCodeNotFound +} + +func (r *paymentOrderLifecycleRedeemRepo) List(context.Context, pagination.PaginationParams) ([]RedeemCode, *pagination.PaginationResult, error) { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) ListWithFilters(context.Context, pagination.PaginationParams, string, string, string) ([]RedeemCode, *pagination.PaginationResult, error) { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) ListByUser(context.Context, int64, int) ([]RedeemCode, error) { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) ListByUserPaginated(context.Context, int64, pagination.PaginationParams, string) ([]RedeemCode, *pagination.PaginationResult, error) { + panic("unexpected call") +} + +func (r *paymentOrderLifecycleRedeemRepo) SumPositiveBalanceByUser(context.Context, int64) (float64, error) { + panic("unexpected call") +} + +func TestVerifyOrderByOutTradeNoBackfillsTradeNoFromPaidQuery(t *testing.T) { + ctx := context.Background() + client := newPaymentOrderLifecycleTestClient(t) + + user, err := client.User.Create(). + SetEmail("checkpaid@example.com"). + SetPasswordHash("hash"). + SetUsername("checkpaid-user"). + Save(ctx) + require.NoError(t, err) + + order, err := client.PaymentOrder.Create(). + SetUserID(user.ID). + SetUserEmail(user.Email). + SetUserName(user.Username). + SetAmount(88). + SetPayAmount(88). + SetFeeRate(0). + SetRechargeCode("CHECKPAID-UPSTREAM-TRADE-NO"). + SetOutTradeNo("sub2_checkpaid_trade_no_missing"). + SetPaymentType(payment.TypeAlipay). + SetPaymentTradeNo(""). + SetOrderType(payment.OrderTypeBalance). + SetStatus(OrderStatusPending). + SetExpiresAt(time.Now().Add(time.Hour)). + SetClientIP("127.0.0.1"). + SetSrcHost("api.example.com"). + Save(ctx) + require.NoError(t, err) + + userRepo := &mockUserRepo{ + getByIDUser: &User{ + ID: user.ID, + Email: user.Email, + Username: user.Username, + Balance: 0, + }, + } + userRepo.updateBalanceFn = func(ctx context.Context, id int64, amount float64) error { + require.Equal(t, user.ID, id) + if userRepo.getByIDUser != nil { + userRepo.getByIDUser.Balance += amount + } + return nil + } + redeemRepo := &paymentOrderLifecycleRedeemRepo{ + codesByCode: map[string]*RedeemCode{ + order.RechargeCode: { + ID: 1, + Code: order.RechargeCode, + Type: RedeemTypeBalance, + Value: order.Amount, + Status: StatusUnused, + }, + }, + } + redeemService := NewRedeemService( + redeemRepo, + userRepo, + nil, + nil, + nil, + client, + nil, + ) + registry := payment.NewRegistry() + provider := &paymentOrderLifecycleQueryProvider{ + resp: &payment.QueryOrderResponse{ + TradeNo: "upstream-trade-123", + Status: payment.ProviderStatusPaid, + Amount: 88, + }, + } + registry.Register(provider) + + svc := &PaymentService{ + entClient: client, + registry: registry, + redeemService: redeemService, + userRepo: userRepo, + providersLoaded: true, + } + + got, err := svc.VerifyOrderByOutTradeNo(ctx, order.OutTradeNo, user.ID) + require.NoError(t, err) + require.Equal(t, order.OutTradeNo, provider.lastQueryTradeNo) + require.Equal(t, OrderStatusCompleted, got.Status) + require.Equal(t, "upstream-trade-123", got.PaymentTradeNo) + + reloaded, err := client.PaymentOrder.Get(ctx, order.ID) + require.NoError(t, err) + require.Equal(t, OrderStatusCompleted, reloaded.Status) + require.Equal(t, "upstream-trade-123", reloaded.PaymentTradeNo) + + require.Equal(t, 88.0, userRepo.getByIDUser.Balance) + require.Len(t, redeemRepo.useCalls, 1) + require.Equal(t, int64(1), redeemRepo.useCalls[0].id) + require.Equal(t, user.ID, redeemRepo.useCalls[0].userID) +} + +func newPaymentOrderLifecycleTestClient(t *testing.T) *dbent.Client { + t.Helper() + + db, err := sql.Open("sqlite", "file:payment_order_lifecycle?mode=memory&cache=shared&_fk=1") + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + _, err = db.Exec("PRAGMA foreign_keys = ON") + require.NoError(t, err) + + drv := entsql.OpenDB(dialect.SQLite, db) + client := enttest.NewClient(t, enttest.WithOptions(dbent.Driver(drv))) + t.Cleanup(func() { _ = client.Close() }) + return client +} diff --git a/frontend/src/api/__tests__/payment.spec.ts b/frontend/src/api/__tests__/payment.spec.ts new file mode 100644 index 00000000..3006484e --- /dev/null +++ b/frontend/src/api/__tests__/payment.spec.ts @@ -0,0 +1,36 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { get, post } = vi.hoisted(() => ({ + get: vi.fn(), + post: vi.fn(), +})) + +vi.mock('@/api/client', () => ({ + apiClient: { + get, + post, + }, +})) + +import { paymentAPI } from '@/api/payment' + +describe('payment api', () => { + beforeEach(() => { + get.mockReset() + post.mockReset() + get.mockResolvedValue({ data: {} }) + post.mockResolvedValue({ data: {} }) + }) + + it('does not expose anonymous public out_trade_no verification', () => { + expect(Object.prototype.hasOwnProperty.call(paymentAPI, 'verifyOrderPublic')).toBe(false) + }) + + it('keeps signed public resume-token resolve endpoint', async () => { + await paymentAPI.resolveOrderPublicByResumeToken('resume-token-123') + + expect(post).toHaveBeenCalledWith('/payment/public/orders/resolve', { + resume_token: 'resume-token-123', + }) + }) +}) diff --git a/frontend/src/api/payment.ts b/frontend/src/api/payment.ts index 91b16866..e866e184 100644 --- a/frontend/src/api/payment.ts +++ b/frontend/src/api/payment.ts @@ -67,11 +67,6 @@ export const paymentAPI = { return apiClient.post('/payment/orders/verify', { out_trade_no: outTradeNo }) }, - /** Verify order payment status without auth (public endpoint for result page) */ - verifyOrderPublic(outTradeNo: string) { - return apiClient.post('/payment/public/orders/verify', { out_trade_no: outTradeNo }) - }, - /** Resolve an order from a signed resume token without auth */ resolveOrderPublicByResumeToken(resumeToken: string) { return apiClient.post('/payment/public/orders/resolve', { resume_token: resumeToken }) diff --git a/frontend/src/views/user/__tests__/PaymentResultView.spec.ts b/frontend/src/views/user/__tests__/PaymentResultView.spec.ts index 56e64793..34ced07a 100644 --- a/frontend/src/views/user/__tests__/PaymentResultView.spec.ts +++ b/frontend/src/views/user/__tests__/PaymentResultView.spec.ts @@ -7,7 +7,6 @@ const routeState = vi.hoisted(() => ({ const routerPush = vi.hoisted(() => vi.fn()) const pollOrderStatus = vi.hoisted(() => vi.fn()) -const verifyOrderPublic = vi.hoisted(() => vi.fn()) const verifyOrder = vi.hoisted(() => vi.fn()) const resolveOrderPublicByResumeToken = vi.hoisted(() => vi.fn()) @@ -38,7 +37,6 @@ vi.mock('@/stores/payment', () => ({ vi.mock('@/api/payment', () => ({ paymentAPI: { - verifyOrderPublic, verifyOrder, resolveOrderPublicByResumeToken, }, @@ -67,7 +65,6 @@ describe('PaymentResultView', () => { routeState.query = {} routerPush.mockReset() pollOrderStatus.mockReset() - verifyOrderPublic.mockReset() verifyOrder.mockReset() resolveOrderPublicByResumeToken.mockReset() window.localStorage.clear() @@ -110,7 +107,6 @@ describe('PaymentResultView', () => { await flushPromises() expect(pollOrderStatus).toHaveBeenCalledWith(42) - expect(verifyOrderPublic).not.toHaveBeenCalled() expect(wrapper.text()).toContain('payment.result.processing') expect(wrapper.text()).not.toContain('payment.result.success') expect(wrapper.text()).not.toContain('payment.result.failed') @@ -221,7 +217,6 @@ describe('PaymentResultView', () => { await flushPromises() expect(resolveOrderPublicByResumeToken).toHaveBeenCalledWith('resume-fail') - expect(verifyOrderPublic).not.toHaveBeenCalled() expect(verifyOrder).not.toHaveBeenCalled() }) @@ -241,7 +236,6 @@ describe('PaymentResultView', () => { await flushPromises() - expect(verifyOrderPublic).not.toHaveBeenCalled() expect(verifyOrder).not.toHaveBeenCalled() }) @@ -260,7 +254,6 @@ describe('PaymentResultView', () => { await flushPromises() - expect(verifyOrderPublic).not.toHaveBeenCalled() expect(verifyOrder).not.toHaveBeenCalled() }) @@ -284,7 +277,6 @@ describe('PaymentResultView', () => { expect(resolveOrderPublicByResumeToken).toHaveBeenCalledWith('resume-77') expect(wrapper.text()).toContain('payment.result.success') - expect(verifyOrderPublic).not.toHaveBeenCalled() }) it('normalizes aliased payment methods before rendering the label', async () => {