diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 0882902..248e939 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -247,6 +247,12 @@ The native Android app is a Capacitor 8 wrapper of the `ui` SPA, scaffolded unde order, and its `test` flag must match the shop's `IsTest` (a test-shop payment must never credit real chips). Answer 200 for anything durably decided, 5xx only to be redelivered (YooKassa retries 24h). + - **Subscribe to `refund.succeeded`, not just the payment events.** The YooKassa cabinet lets an + operator refund without touching our API, so without that event the money goes back while the + chips stay credited. The reversal engine is **full-refund-only** (it rejects any amount other than + the order's), so a partial refund must record nothing and be logged for a human — there is no + non-arbitrary chip cost for a part-refund. Also: a refund is only recorded in status `succeeded`; + a `pending` one can still be canceled and the ledger is append-only. - **YooKassa DOES have a test mode** — a separate *test shop* (own shopId/secret, test cards, up to 20 of them), and webhooks/receipts/refunds all work there. The whole rail is verifiable on the contour without real money; don't plan around "no test payments". diff --git a/PLAN.md b/PLAN.md index 9ccf629..22a3953 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1038,8 +1038,11 @@ when no YooKassa shop is configured, so reviving it is a credentials change (D47 records a `failed` event. 200 = durably decided, 5xx = redeliver. - **Reconcile** — `runOrderReaper` asks the provider about each pending order that reached its expiry age carrying a payment id, and credits the ones really paid (D49). One request per order. -- **Refund** — the `/_gm` button calls `POST /v3/refunds` first and records only on success, under the - provider's own refund id (D50); a failure records nothing. +- **Refund** — the `/_gm` button calls `POST /v3/refunds` first and records only on a **succeeded** + refund, under the provider's own refund id (D50); a failure — or a still-`pending` refund — records + nothing, and pressing again is safe. `refund.succeeded` is handled too (D52), because the merchant + cabinet can issue a refund that never passes through our API; a **partial** refund records nothing + and is logged for an operator (the engine is full-refund-only). - **Fiscalization** — «Чеки от ЮKassa» with the `receipt` sent from code (D51), reversing E10's B4: YooKassa has no cabinet-side generic receipt. `BACKEND_YOOKASSA_VAT_CODE` is a deploy variable; the settlement subject/method are constants. @@ -1052,7 +1055,9 @@ when no YooKassa shop is configured, so reviving it is a credentials change (D47 forged notification credits nothing, a foreign sender is refused, a live payment on a test shop is refused, a decline records `failed`, the reconcile sweep credits a lost notification and leaves an unpaid order alone, the refund moves money then records — and records nothing when it fails, the - order path mints a payment with a receipt, and D36 still gates the rail). + order path mints a payment with a receipt, D36 still gates the rail, a cabinet refund is reversed + once, the event after a console refund is a no-op, a partial refund changes nothing, an unconfirmed + refund reverses nothing, and a pending refund records nothing until it settles). **Contour-safe:** no migration, no wire change; the client is rail-agnostic (only the mock URL and an e2e assertion name a provider). diff --git a/backend/internal/inttest/payments_yookassa_test.go b/backend/internal/inttest/payments_yookassa_test.go index 67e83a9..34c00bf 100644 --- a/backend/internal/inttest/payments_yookassa_test.go +++ b/backend/internal/inttest/payments_yookassa_test.go @@ -37,8 +37,12 @@ type fakeYooKassa struct { payments map[string]yookassa.Payment // refundFails makes POST /refunds answer 500, standing in for a provider that did not move money. refundFails bool + // refundStatus is the status a created refund reports; empty means succeeded. + refundStatus string // refundCalls counts the refunds actually requested. refundCalls int + // refunds answers GET /refunds/{id}; a missing id is a 404. + refunds map[string]yookassa.Refund // createdIdempotenceKey records the key sent on the last create-payment call. createdIdempotenceKey string // lastReceipt records the receipt object of the last create-payment call. @@ -48,7 +52,7 @@ type fakeYooKassa struct { // newFakeYooKassa starts the fake API and returns it with a shop config pointed at it. func newFakeYooKassa(t *testing.T) (*fakeYooKassa, yookassa.Config) { t.Helper() - f := &fakeYooKassa{payments: map[string]yookassa.Payment{}} + f := &fakeYooKassa{payments: map[string]yookassa.Payment{}, refunds: map[string]yookassa.Refund{}} srv := httptest.NewServer(http.HandlerFunc(f.serve)) t.Cleanup(srv.Close) return f, yookassa.Config{ShopID: ykShopID, SecretKey: ykSecretKey, IsTest: true, BaseURL: srv.URL} @@ -94,14 +98,37 @@ func (f *fakeYooKassa) serve(w http.ResponseWriter, r *http.Request) { f.refundCalls++ var body yookassa.RefundRequest _ = json.NewDecoder(r.Body).Decode(&body) - _ = json.NewEncoder(w).Encode(yookassa.Refund{ - ID: "refund-1", Status: yookassa.StatusSucceeded, PaymentID: body.PaymentID, Amount: body.Amount, - }) + status := f.refundStatus + if status == "" { + status = yookassa.StatusSucceeded + } + // The ledger dedupes refunds on (provider, refund id) across the whole database, so the id has + // to be unique per payment or one test's refund looks like another's duplicate. + refund := yookassa.Refund{ID: "refund-" + body.PaymentID, Status: status, PaymentID: body.PaymentID, Amount: body.Amount} + f.refunds[refund.ID] = refund + _ = json.NewEncoder(w).Encode(refund) + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/refunds/"): + refund, ok := f.refunds[strings.TrimPrefix(r.URL.Path, "/refunds/")] + if !ok { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"type":"error","description":"not found"}`)) + return + } + _ = json.NewEncoder(w).Encode(refund) default: w.WriteHeader(http.StatusNotFound) } } +// setRefund publishes a refund the API will report — how a test stands up a refund that was issued +// somewhere other than through our own API, such as in the merchant cabinet. +func (f *fakeYooKassa) setRefund(id string, refund yookassa.Refund) { + f.mu.Lock() + defer f.mu.Unlock() + refund.ID = id + f.refunds[id] = refund +} + // setPayment overwrites what the API reports for a payment — how a test says "the money did (or did // not) really move", independently of what a notification claims. func (f *fakeYooKassa) setPayment(id string, mutate func(p *yookassa.Payment)) { @@ -383,8 +410,8 @@ func TestYooKassaConsoleRefundMovesMoneyThenRecords(t *testing.T) { acc).Scan(&provider, &refundID); err != nil { t.Fatalf("read refund ledger row: %v", err) } - if provider != "yookassa" || refundID != "refund-1" { - t.Errorf("refund ledger row = (%s, %s), want (yookassa, refund-1)", provider, refundID) + if provider != "yookassa" || refundID != "refund-"+paymentID { + t.Errorf("refund ledger row = (%s, %s), want (yookassa, refund-%s)", provider, refundID, paymentID) } } @@ -502,3 +529,182 @@ func TestYooKassaOrderRequiresAnEmailAnchor(t *testing.T) { t.Fatalf("order = %d (%s), want 403 email_required", rec.Code, rec.Body.String()) } } + +// postYooKassaRefundNotify posts a refund notification to the intake and reports the status code. +func postYooKassaRefundNotify(t *testing.T, srv *server.Server, refundID, paymentID string) int { + t.Helper() + body := fmt.Sprintf(`{"type":"notification","event":%q,"object":{"id":%q,"status":"succeeded","payment_id":%q}}`, + yookassa.EventRefundSucceeded, refundID, paymentID) + req := httptest.NewRequest(http.MethodPost, "/api/v1/internal/payments/yookassa/notify", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", ykSenderIP) + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + return rec.Code +} + +// fundedYooKassaOrder seeds an order and credits it, leaving an account with chips to reverse. +func fundedYooKassaOrder(t *testing.T, f *fakeYooKassa, srv *server.Server, pay *payments.Service, acc uuid.UUID) (uuid.UUID, string) { + t.Helper() + orderID, paymentID := seedYooKassaOrder(t, f, pay, acc) + if code := postYooKassaNotify(t, srv, ykSenderIP, yookassa.EventPaymentSucceeded, paymentID, orderID.String()); code != http.StatusOK { + t.Fatalf("notify = %d, want 200", code) + } + return orderID, paymentID +} + +// TestYooKassaCabinetRefundIsReversed is why the refund event is handled at all: an operator can +// refund in the merchant cabinet, which never passes through our API, so without this the money would +// go back while the chips stayed credited. +func TestYooKassaCabinetRefundIsReversed(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + _, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc) + if got := readBalance(t, acc, "direct"); got != 100 { + t.Fatalf("balance before the refund = %d, want 100", got) + } + + // The refund exists at the provider, issued outside our API. + f.setRefund("cabinet-refund-1", yookassa.Refund{ + Status: yookassa.StatusSucceeded, PaymentID: paymentID, + Amount: yookassa.Amount{Value: "149.00", Currency: "RUB"}, + }) + if code := postYooKassaRefundNotify(t, srv, "cabinet-refund-1", paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + if got := readBalance(t, acc, "direct"); got != 0 { + t.Errorf("balance = %d, want 0 — a cabinet refund did not revoke the chips", got) + } + if f.refundCalls != 0 { + t.Errorf("provider refunds requested = %d, want 0 — the money had already moved", f.refundCalls) + } + var provider, refundID string + if err := testDB.QueryRowContext(context.Background(), + `SELECT provider, provider_payment_id FROM payments.ledger WHERE account_id=$1 AND kind='refund'`, + acc).Scan(&provider, &refundID); err != nil { + t.Fatalf("read refund ledger row: %v", err) + } + if provider != "yookassa" || refundID != "cabinet-refund-1" { + t.Errorf("refund ledger row = (%s, %s), want (yookassa, cabinet-refund-1)", provider, refundID) + } + + // A redelivery reverses nothing a second time. + if code := postYooKassaRefundNotify(t, srv, "cabinet-refund-1", paymentID); code != http.StatusOK { + t.Fatalf("redelivered refund notify = %d, want 200", code) + } + if ledgerRows(t, acc, "refund") != 1 { + t.Errorf("refund ledger rows = %d, want 1", ledgerRows(t, acc, "refund")) + } +} + +// TestYooKassaRefundNotifyAfterConsoleRefundIsANoOp checks the two refund entry points cannot double +// up: the event for a refund the console already recorded reverses nothing again. +func TestYooKassaRefundNotifyAfterConsoleRefundIsANoOp(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + orderID, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc) + + base := "http://admin.test/_gm/users/" + acc.String() + if code, _ := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test"); code != http.StatusOK { + t.Fatalf("console refund = %d, want 200", code) + } + if got := readBalance(t, acc, "direct"); got != 0 { + t.Fatalf("balance after the console refund = %d, want 0", got) + } + // YooKassa then notifies about that same refund. + if code := postYooKassaRefundNotify(t, srv, "refund-"+paymentID, paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + if ledgerRows(t, acc, "refund") != 1 { + t.Errorf("refund ledger rows = %d, want 1 (reversed once)", ledgerRows(t, acc, "refund")) + } + if abuse, loss := readRisk(t, acc); abuse || loss != 0 { + t.Errorf("risk = (abuse %v, loss %d), want none — the second reversal ran anyway", abuse, loss) + } +} + +// TestYooKassaPartialRefundIsLeftToAnOperator: the reversal engine revokes exactly what the pack +// funded and rejects any other amount, so a partial refund cannot be recorded without guessing how +// many chips it costs. It must change nothing rather than guess. +func TestYooKassaPartialRefundIsLeftToAnOperator(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + _, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc) + + f.setRefund("partial-1", yookassa.Refund{ + Status: yookassa.StatusSucceeded, PaymentID: paymentID, + Amount: yookassa.Amount{Value: "50.00", Currency: "RUB"}, // part of the 149.00 order + }) + if code := postYooKassaRefundNotify(t, srv, "partial-1", paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + if got := readBalance(t, acc, "direct"); got != 100 { + t.Errorf("balance = %d, want 100 — a partial refund revoked chips", got) + } + if ledgerRows(t, acc, "refund") != 0 { + t.Error("a partial refund wrote a refund ledger row") + } +} + +// TestYooKassaRefundNotifyIsNotEvidence: as with a payment, the body only names a refund. One the +// provider does not confirm reverses nothing. +func TestYooKassaRefundNotifyIsNotEvidence(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + _, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc) + + // A refund the provider has never heard of. + if code := postYooKassaRefundNotify(t, srv, "invented-refund", paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + // One that exists but is not final. + f.setRefund("pending-refund", yookassa.Refund{ + Status: yookassa.StatusPending, PaymentID: paymentID, + Amount: yookassa.Amount{Value: "149.00", Currency: "RUB"}, + }) + if code := postYooKassaRefundNotify(t, srv, "pending-refund", paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + if got := readBalance(t, acc, "direct"); got != 100 { + t.Errorf("balance = %d, want 100 — an unconfirmed refund revoked chips", got) + } + if ledgerRows(t, acc, "refund") != 0 { + t.Error("an unconfirmed refund wrote a refund ledger row") + } +} + +// TestYooKassaPendingRefundRecordsNothing: a refund can still be canceled while pending and the +// ledger is append-only, so the console records only a completed one. Retrying is the way out. +func TestYooKassaPendingRefundRecordsNothing(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + orderID, _ := fundedYooKassaOrder(t, f, srv, pay, acc) + + f.refundStatus = yookassa.StatusPending + base := "http://admin.test/_gm/users/" + acc.String() + _, body := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test") + if !strings.Contains(body, "not completed the refund yet") || !strings.Contains(body, "nothing was recorded") { + t.Errorf("message = %q, want it to say the refund is not final and nothing was recorded", body) + } + if got := readBalance(t, acc, "direct"); got != 100 { + t.Errorf("balance = %d, want 100 — chips were revoked for an unsettled refund", got) + } + if ledgerRows(t, acc, "refund") != 0 { + t.Error("a pending refund wrote a refund ledger row") + } + + // Once the provider settles it, pressing again records the reversal — the idempotency key returns + // the same refund rather than paying twice. + f.refundStatus = yookassa.StatusSucceeded + if code, body := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test"); code != http.StatusOK || !strings.Contains(body, "revoked 100 chips") { + t.Fatalf("retried refund = %d, body has 'revoked 100 chips' = %v", code, strings.Contains(body, "revoked 100 chips")) + } + if got := readBalance(t, acc, "direct"); got != 0 { + t.Errorf("balance = %d, want 0", got) + } +} diff --git a/backend/internal/payments/service_intake.go b/backend/internal/payments/service_intake.go index ab3c53b..d76e60d 100644 --- a/backend/internal/payments/service_intake.go +++ b/backend/internal/payments/service_intake.go @@ -113,6 +113,16 @@ func (s *Service) OrderProviderRef(ctx context.Context, orderID uuid.UUID) (Orde return orderRef(o) } +// OrderByProviderPayment reads the order a provider's payment id belongs to. It resolves a provider +// event that names only its own payment — such as a refund notification — back to an order. +func (s *Service) OrderByProviderPayment(ctx context.Context, provider, providerPaymentID string) (OrderRef, error) { + o, err := s.store.orderByProviderPayment(ctx, provider, providerPaymentID) + if err != nil { + return OrderRef{}, err + } + return orderRef(o) +} + // reconcileBatch bounds one reconcile sweep, so a backlog cannot turn a periodic tick into a long // run of provider calls. const reconcileBatch = 50 diff --git a/backend/internal/payments/store_intake.go b/backend/internal/payments/store_intake.go index 63b8eb3..f7f6e32 100644 --- a/backend/internal/payments/store_intake.go +++ b/backend/internal/payments/store_intake.go @@ -229,6 +229,29 @@ func derefString(p *string) string { return *p } +// orderByProviderPayment reads the order a provider's payment id belongs to, or ErrOrderNotFound. +// It is how a provider event that names only its own payment — a refund notification, say — is +// resolved back to an order. +func (s *Store) orderByProviderPayment(ctx context.Context, provider, providerPaymentID string) (orderRow, error) { + if provider == "" || providerPaymentID == "" { + return orderRow{}, ErrOrderNotFound + } + var o model.Orders + err := postgres.SELECT(table.Orders.AllColumns). + FROM(table.Orders). + WHERE(table.Orders.Provider.EQ(postgres.String(provider)). + AND(table.Orders.ProviderPaymentID.EQ(postgres.String(providerPaymentID)))). + LIMIT(1). + QueryContext(ctx, s.db, &o) + if errors.Is(err, qrm.ErrNoRows) { + return orderRow{}, ErrOrderNotFound + } + if err != nil { + return orderRow{}, fmt.Errorf("payments: load order by %s payment %s: %w", provider, providerPaymentID, err) + } + return newOrderRow(o), nil +} + // attachProviderPayment records the provider's own payment id on a pending order, as soon as the // provider mints it. It is what later lets an unattended order be re-checked against the provider // and a refund address the right payment; fund overwrites it with the same value when the callback diff --git a/backend/internal/server/handlers_admin_refund.go b/backend/internal/server/handlers_admin_refund.go index ccbd2ec..fb43156 100644 --- a/backend/internal/server/handlers_admin_refund.go +++ b/backend/internal/server/handlers_admin_refund.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/csv" + "errors" "fmt" "strconv" "strings" @@ -71,6 +72,12 @@ func (s *Server) refundOrder(ctx context.Context, ref payments.OrderRef) (paymen return s.payments.RefundOrderFull(ctx, ref.OrderID) } refundID, err := s.refundYooKassa(ctx, ref) + if errors.Is(err, errRefundNotFinal) { + // The money is on its way but not settled. Nothing is recorded yet; pressing again is safe — + // the idempotency key returns the same refund rather than paying a second time. + s.log.Warn("yookassa refund not final", zap.String("order", ref.OrderID.String()), zap.Error(err)) + return payments.RefundOutcome{}, fmt.Errorf("%w — nothing was recorded; try again in a minute", err) + } if err != nil { s.log.Error("yookassa refund failed", zap.String("order", ref.OrderID.String()), zap.Error(err)) return payments.RefundOutcome{}, fmt.Errorf("the provider did not refund the payment, nothing was recorded: %w", err) diff --git a/backend/internal/server/handlers_yookassa.go b/backend/internal/server/handlers_yookassa.go index 6f4ce40..1e0ef1a 100644 --- a/backend/internal/server/handlers_yookassa.go +++ b/backend/internal/server/handlers_yookassa.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "strings" @@ -111,9 +112,11 @@ func (s *Server) handleYooKassaNotify(c *gin.Context) { } switch note.Event { case yookassa.EventPaymentSucceeded, yookassa.EventPaymentCanceled: + case yookassa.EventRefundSucceeded: + s.handleYooKassaRefundNotification(c, note) + return default: - // Refund events are informational: refunds are issued through the API, which records them - // synchronously. Anything else is a subscription we do not act on. + // A subscription we do not act on. c.String(http.StatusOK, "ignored") return } @@ -189,6 +192,95 @@ func (s *Server) handleYooKassaNotify(c *gin.Context) { c.String(http.StatusOK, "ok") } +// handleYooKassaRefundNotification reverses a refund the provider reports as completed. Its reason +// for existing is the merchant cabinet: an operator can refund there, and such a refund never passes +// through our API, so without this the money would go back while the chips stayed credited. +// +// As with a payment, the body is only a hint — the refund is re-read from the API before anything is +// recorded. The reversal is idempotent on (yookassa, refund id), so the notification for a refund the +// console already recorded reverses nothing a second time. +func (s *Server) handleYooKassaRefundNotification(c *gin.Context, note yookassa.Notification) { + claimed, err := note.Refund() + if err != nil { + s.log.Warn("yookassa notify: unusable refund object", zap.Error(err)) + c.String(http.StatusOK, "ignored") + return + } + ctx := c.Request.Context() + // The refund object names the payment, not our order, so the order is found by the payment id we + // recorded when the payment was minted. The claim is untrusted, but it only selects which shop's + // credentials perform the confirming read; the confirmed refund is then bound back to this order. + ref, err := s.payments.OrderByProviderPayment(ctx, providerYooKassa, claimed.PaymentID) + if errors.Is(err, payments.ErrOrderNotFound) { + s.log.Warn("yookassa refund notify: no order for the payment", + zap.String("payment", claimed.PaymentID), zap.String("refund", claimed.ID)) + c.String(http.StatusOK, "ignored") + return + } + if err != nil { + s.log.Error("yookassa refund notify: order lookup failed", zap.String("payment", claimed.PaymentID), zap.Error(err)) + c.String(http.StatusInternalServerError, "error") + return + } + shop, ok := s.yookassa.Shop(ref.Shop) + if !ok { + s.log.Warn("yookassa refund notify: no shop for the order's channel", zap.String("order", ref.OrderID.String())) + c.String(http.StatusOK, "ignored") + return + } + refund, err := shop.GetRefund(ctx, claimed.ID) + if err != nil { + var apiErr *yookassa.APIError + if errors.As(err, &apiErr) && !apiErr.Retryable() { + s.log.Warn("yookassa refund notify: refund not confirmed", + zap.String("order", ref.OrderID.String()), zap.String("refund", claimed.ID), zap.Error(err)) + c.String(http.StatusOK, "ignored") + return + } + s.log.Error("yookassa refund notify: confirm failed", + zap.String("order", ref.OrderID.String()), zap.String("refund", claimed.ID), zap.Error(err)) + c.String(http.StatusInternalServerError, "error") + return + } + if refund.Status != yookassa.StatusSucceeded || refund.PaymentID != ref.PaymentID { + s.log.Warn("yookassa refund notify: confirmed refund does not settle this order", + zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID), + zap.String("status", refund.Status), zap.String("refund_payment", refund.PaymentID)) + c.String(http.StatusOK, "ignored") + return + } + // The reversal engine is full-refund-only by design: it revokes exactly what the pack funded and + // rejects any other amount. A partial refund therefore cannot be recorded without guessing how + // many chips it should cost, so it is left to an operator — loudly, because the money has moved. + paid, err := payments.ParseMoney(refund.Amount.Value, payments.Currency(refund.Amount.Currency)) + if err != nil || paid.Currency() != ref.Amount.Currency() || paid.Minor() != ref.Amount.Minor() { + s.log.Error("yookassa refund notify: partial refund needs an operator; nothing was reversed", + zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID), + zap.String("refunded", refund.Amount.Value), zap.String("order_amount", ref.Amount.Major())) + c.String(http.StatusOK, "ignored") + return + } + out, err := s.payments.RefundOrderFullAs(ctx, ref.OrderID, providerYooKassa, refund.ID) + if err != nil { + if errors.Is(err, payments.ErrOrderNotPaid) { + s.log.Warn("yookassa refund notify: the order was never credited", + zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID)) + c.String(http.StatusOK, "ignored") + return + } + s.log.Error("yookassa refund notify: reversal failed", + zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID), zap.Error(err)) + c.String(http.StatusInternalServerError, "error") + return + } + if !out.AlreadyRefunded { + s.log.Info("yookassa refund notify: reversed a refund issued outside the console", + zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID), + zap.Int("revoked", out.Revoked), zap.Int("loss", out.Loss)) + } + c.String(http.StatusOK, "ok") +} + // confirmYooKassaPayment re-reads a payment from the API — the authenticity check for the whole rail, // since notifications are unsigned. The shop is chosen by the shop id the payment claims to have been // paid to, falling back to the merchant channel recorded on the order; a claim naming a shop we do @@ -305,9 +397,20 @@ func (s *Server) refundYooKassa(ctx context.Context, ref payments.OrderRef) (str if refund.ID == "" { return "", errors.New("the provider returned a refund with no id") } + // Only a completed refund is recorded. A refund can still be canceled while pending, and the + // ledger is append-only, so recording early would mean revoking a customer's chips for money that + // then stayed with us — with no way to take the row back. + if refund.Status != yookassa.StatusSucceeded { + return "", fmt.Errorf("%w (status %s)", errRefundNotFinal, refund.Status) + } return refund.ID, nil } +// errRefundNotFinal reports that the provider accepted the refund but has not completed it. Retrying +// is safe and is the way out: the idempotency key returns the same refund rather than paying twice, +// so the operator can press again until it settles. +var errRefundNotFinal = errors.New("the provider has not completed the refund yet") + // ReconcileYooKassaOrders asks YooKassa what became of every pending order that reached its expiry // age while carrying a payment id, and credits the ones that were in fact paid. It is the safety net // for a notification that was lost for good: YooKassa redelivers for 24 hours, so a short outage diff --git a/backend/internal/yookassa/notify.go b/backend/internal/yookassa/notify.go index d3b2304..1d174b3 100644 --- a/backend/internal/yookassa/notify.go +++ b/backend/internal/yookassa/notify.go @@ -14,8 +14,9 @@ const ( // EventPaymentCanceled means the payment was actively declined or abandoned; nothing is credited // and the payer is told the attempt failed. EventPaymentCanceled = "payment.canceled" - // EventRefundSucceeded reports a completed refund. Refunds here are always initiated by an - // operator through the API, which records them synchronously, so this event is informational. + // EventRefundSucceeded reports a completed refund. It matters because the merchant cabinet is a + // second way to issue one: a refund made there never passes through our API, so without this + // event the money would go back while the chips stayed credited. EventRefundSucceeded = "refund.succeeded" ) @@ -58,6 +59,19 @@ func (n Notification) Payment() (Payment, error) { return p, nil } +// Refund decodes the notification's object as a refund. Use it only for refund.* events; it yields +// the refund id to re-read, never the refund state to act on. +func (n Notification) Refund() (Refund, error) { + var r Refund + if err := json.Unmarshal(n.Object, &r); err != nil { + return Refund{}, fmt.Errorf("yookassa: decode notification refund: %w", err) + } + if r.ID == "" { + return Refund{}, fmt.Errorf("yookassa: notification refund has no id") + } + return r, nil +} + // senderPrefixes are the address ranges YooKassa delivers notifications from // (https://yookassa.ru/developers/using-api/webhooks). Single addresses are expressed as /32 and // /128 prefixes. The list is defence in depth only — the confirming GetPayment is what actually diff --git a/backend/internal/yookassa/notify_test.go b/backend/internal/yookassa/notify_test.go index 5a6050c..69bca13 100644 --- a/backend/internal/yookassa/notify_test.go +++ b/backend/internal/yookassa/notify_test.go @@ -88,3 +88,33 @@ func TestAllowedSenderUnmapsIPv4(t *testing.T) { t.Error("an IPv4-mapped foreign sender was allowed") } } + +func TestParseRefundNotification(t *testing.T) { + n, err := ParseNotification([]byte(`{"type":"notification","event":"refund.succeeded", + "object":{"id":"refund-1","status":"succeeded","payment_id":"pay-1", + "amount":{"value":"149.00","currency":"RUB"}}}`)) + if err != nil { + t.Fatalf("parse notification: %v", err) + } + if n.Event != EventRefundSucceeded { + t.Errorf("event = %q, want %q", n.Event, EventRefundSucceeded) + } + r, err := n.Refund() + if err != nil { + t.Fatalf("decode notification refund: %v", err) + } + // The refund names the payment, not our order — that is how it is resolved back to one. + if r.ID != "refund-1" || r.PaymentID != "pay-1" { + t.Errorf("refund = %+v, want refund-1 for pay-1", r) + } +} + +func TestNotificationRefundNeedsAnID(t *testing.T) { + n, err := ParseNotification([]byte(`{"type":"notification","event":"refund.succeeded","object":{"status":"succeeded"}}`)) + if err != nil { + t.Fatalf("parse notification: %v", err) + } + if _, err := n.Refund(); err == nil { + t.Error("a refund object with no id was accepted") + } +} diff --git a/backend/internal/yookassa/yookassa.go b/backend/internal/yookassa/yookassa.go index 0f324f3..f8e8b33 100644 --- a/backend/internal/yookassa/yookassa.go +++ b/backend/internal/yookassa/yookassa.go @@ -207,13 +207,22 @@ func (c Config) GetPayment(ctx context.Context, paymentID string) (Payment, erro } // CreateRefund returns money for a succeeded payment and reports the created refund. idempotenceKey -// guards against a double refund on a retry; callers pass the order id. +// guards against a double refund on a retry; callers pass the order id. The returned refund is not +// necessarily final — check Status before acting on it. func (c Config) CreateRefund(ctx context.Context, req RefundRequest, idempotenceKey string) (Refund, error) { var out Refund err := c.do(ctx, http.MethodPost, "/refunds", req, idempotenceKey, &out) return out, err } +// GetRefund re-reads a refund by id. As with GetPayment this is the authenticity check: a refund +// notification is unsigned, so only the object returned here may be acted on. +func (c Config) GetRefund(ctx context.Context, refundID string) (Refund, error) { + var out Refund + err := c.do(ctx, http.MethodGet, "/refunds/"+url.PathEscape(refundID), nil, "", &out) + return out, err +} + // do performs one authenticated API call and decodes the result into out. A non-2xx answer is // returned as an *APIError carrying YooKassa's own error object when the body holds one. func (c Config) do(ctx context.Context, method, path string, body any, idempotenceKey string, out any) error { diff --git a/backend/internal/yookassa/yookassa_test.go b/backend/internal/yookassa/yookassa_test.go index 8d20b11..e556b0d 100644 --- a/backend/internal/yookassa/yookassa_test.go +++ b/backend/internal/yookassa/yookassa_test.go @@ -223,3 +223,38 @@ func TestEndpointDefaultsToTheProductionAPI(t *testing.T) { t.Errorf("endpoint = %q, want the trailing slash trimmed", got) } } + +func TestGetRefundReadsTheObject(t *testing.T) { + cfg, got := fakeAPI(t, http.StatusOK, `{ + "id":"refund-1","status":"succeeded","payment_id":"pay-1", + "amount":{"value":"149.00","currency":"RUB"}}`) + + r, err := cfg.GetRefund(context.Background(), "refund-1") + if err != nil { + t.Fatalf("get refund: %v", err) + } + if got.method != http.MethodGet || got.path != "/refunds/refund-1" { + t.Errorf("request = %s %s, want GET /refunds/refund-1", got.method, got.path) + } + if r.Status != StatusSucceeded || r.PaymentID != "pay-1" { + t.Errorf("refund = %+v, want a succeeded refund of pay-1", r) + } +} + +func TestCreateRefundReportsANonFinalStatus(t *testing.T) { + // A refund can be accepted and still be canceled later, so the caller must be able to see that it + // is not settled rather than treat any 200 as done. + cfg, _ := fakeAPI(t, http.StatusOK, `{ + "id":"refund-1","status":"pending","payment_id":"pay-1", + "amount":{"value":"149.00","currency":"RUB"}}`) + + r, err := cfg.CreateRefund(context.Background(), RefundRequest{ + PaymentID: "pay-1", Amount: Amount{Value: "149.00", Currency: "RUB"}, + }, "0197-order") + if err != nil { + t.Fatalf("create refund: %v", err) + } + if r.Status != StatusPending { + t.Errorf("status = %q, want pending surfaced to the caller", r.Status) + } +} diff --git a/deploy/.env.example b/deploy/.env.example index 9678aa3..0dfddb5 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -140,7 +140,8 @@ GATEWAY_VK_ID_CLIENT_SECRET= # to credit a live payment against it (and a test payment against a live shop). # Mapped in compose to BACKEND_YOOKASSA_*; Gitea TEST_/PROD_ secrets. # Set each shop's notification URL in its cabinet (Интеграция — HTTP-уведомления) to -# ${PUBLIC_BASE_URL}/pay/yookassa/notify, events payment.succeeded + payment.canceled. +# ${PUBLIC_BASE_URL}/pay/yookassa/notify, events payment.succeeded + payment.canceled + +# refund.succeeded (the last one catches a refund issued in the cabinet, which bypasses our API). YOOKASSA_WEB_SHOP_ID= YOOKASSA_WEB_SECRET_KEY= YOOKASSA_WEB_TEST= diff --git a/deploy/README.md b/deploy/README.md index 8175f98..8f3946d 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -76,7 +76,7 @@ compose binds from this directory. | `GM_BASICAUTH_HASH` | secret | bcrypt hash gating `/_gm` (admin console + Grafana). Generate with `docker run --rm caddy:2-alpine caddy hash-password --plaintext ''`. | | `TELEGRAM_MINIAPP_URL` | derived | The Mini App URL the bot hands out in deep links / buttons. The deploy derives `PUBLIC_BASE_URL + /telegram/`; set it directly only for a local run (compose still `:?`-requires it). | | `EXPORT_SIGN_KEY` | secret | HMAC key signing the public finished-game export download URLs (`/dl/*`). Generate with `openssl rand -base64 32`. | -| `BACKEND_YOOKASSA_{WEB,ANDROID}_SHOP_ID` | secret | YooKassa shop id for that channel of the direct RUB rail (cabinet: **Настройки — Магазин**, field `shopId`). Empty ⇒ that channel is unconfigured and order routing falls back to the **web** shop; no channel configured at all leaves the rail off. | +| `BACKEND_YOOKASSA_{WEB,ANDROID}_SHOP_ID` | secret | YooKassa shop id for that channel of the direct RUB rail (cabinet: **Настройки — Магазин**, field `shopId`). Empty ⇒ that channel is unconfigured and order routing falls back to the **web** shop; no channel configured at all leaves the rail off. Each shop's **Интеграция — HTTP-уведомления** must point at `/pay/yookassa/notify` with events `payment.succeeded`, `payment.canceled` and `refund.succeeded`. | | `BACKEND_YOOKASSA_{WEB,ANDROID}_SECRET_KEY` | secret | That shop's API secret key (cabinet: **Интеграция — Ключи API**). It is the password half of the HTTP Basic credentials on every API call. A shop id without a secret key is ignored. | | `BACKEND_YOOKASSA_{WEB,ANDROID}_TEST` | **variable** | `1` marks the credentials as a **test shop**: the intake then refuses to credit anything but a test payment (and a live shop refuses a test one). A variable, not a secret, so switching a contour between a test and a live shop is a flag flip + redeploy. | | `BACKEND_YOOKASSA_VAT_CODE` | **variable** | VAT rate code stamped on every fiscal receipt line (54-ФЗ tag 1199): `1` = «Без НДС» (УСН/ПСН), `4` = 20%, `11` = 22%. Defaults to `1`. A variable so a rate change needs no code change. **Confirm it with the accountant before the first real payment** — a wrong rate produces wrong receipts, not a failed payment, so it fails silently. | diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 0c90a70..24379d7 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -169,7 +169,8 @@ services: # Each shop credits the one direct wallet; a shop missing either credential drops that # channel (order routing falls back to the web shop), and no shop at all leaves the direct # order and notification endpoints unregistered. Notification URL (set per shop in the - # YooKassa cabinet): ${PUBLIC_BASE_URL}/pay/yookassa/notify. + # YooKassa cabinet): ${PUBLIC_BASE_URL}/pay/yookassa/notify — events payment.succeeded, + # payment.canceled and refund.succeeded. BACKEND_YOOKASSA_WEB_SHOP_ID: ${YOOKASSA_WEB_SHOP_ID:-} BACKEND_YOOKASSA_WEB_SECRET_KEY: ${YOOKASSA_WEB_SECRET_KEY:-} BACKEND_YOOKASSA_WEB_TEST: ${YOOKASSA_WEB_TEST:-} diff --git a/docs/PAYMENTS.md b/docs/PAYMENTS.md index d21f384..295eabc 100644 --- a/docs/PAYMENTS.md +++ b/docs/PAYMENTS.md @@ -303,14 +303,28 @@ an abandoned pending) is surfaced to the user; "payment succeeded" is a hook (em message). **Refunds.** ToS is **non-refundable** — we do not offer refunds to the user. Refunds are -**admin-triggered** (the E7 console), since no rail pushes an unsolicited refund. On the direct -rail the console does the whole job in one click (D50): it calls YooKassa's refund API first -(`POST /v3/refunds`, the order id as the `Idempotence-Key`, carrying the refund receipt of §12) and -records the reversal only once the money has actually moved — a failed call records **nothing**, so -the ledger can never claim a refund that did not happen. The recorded refund id is the provider's -own, which keeps the ledger reconcilable against YooKassa's records. VK refunds are still handled by -support and Telegram Stars refunds issued with `refundStarPayment`, both recorded by hand afterwards. -All of them converge on one engine — +**admin-triggered** (the E7 console). On the direct rail the console does the whole job in one click +(D50): it calls YooKassa's refund API first (`POST /v3/refunds`, the order id as the +`Idempotence-Key`, carrying the refund receipt of §12) and records the reversal only once the money +has actually moved — a failed call records **nothing**, so the ledger can never claim a refund that +did not happen. A refund is recorded **only in status `succeeded`**: one still `pending` can yet be +canceled, and the ledger is append-only, so recording early could revoke a customer's chips for money +that stayed with us. Pressing again is the way out — the idempotency key returns the same refund +rather than paying twice. The recorded refund id is the provider's own, which keeps the ledger +reconcilable against YooKassa's records. + +**The cabinet is a second entry point (D52).** Unlike the earlier rails, YooKassa lets an operator +refund from the merchant cabinet, and such a refund never passes through our API — so the money would +go back while the chips stayed credited. The `refund.succeeded` notification closes that: the refund +is re-read from the API (the body is no more evidence than a payment notification's), bound back to +the order through the payment id we recorded, and reversed through the same engine — idempotent on +`(provider, refund id)`, so the notification for a refund the console already recorded reverses +nothing twice. The engine is **full-refund-only** by design (it revokes exactly what the pack funded +and rejects any other amount), so a **partial** refund is recorded as nothing at all and logged +loudly for an operator: there is no non-arbitrary way to decide how many chips a part-refund costs. + +VK refunds are still handled by support and Telegram Stars refunds issued with `refundStarPayment`, +both recorded by hand afterwards. All of them converge on one engine — the `Refund` method (`internal/payments`): it matches the paid order, appends a **refund** ledger row (idempotent on `(provider, provider_refund_id)` — the refund id is distinct from the fund's payment id, so the two rows coexist under the same partial-unique index), and **best-effort revokes diff --git a/docs/PAYMENTS_DECISIONS_ru.md b/docs/PAYMENTS_DECISIONS_ru.md index 17bcc25..63283d9 100644 --- a/docs/PAYMENTS_DECISIONS_ru.md +++ b/docs/PAYMENTS_DECISIONS_ru.md @@ -309,8 +309,20 @@ web+PWA / native Android+iOS через Capacitor). Владелец (самоз - **D50. Возвраты на direct-рельсе — через API ЮKassa, одним действием.** Кнопка возврата в `/_gm` сперва двигает деньги (`POST /v3/refunds`, `Idempotence-Key` = идентификатор заказа, с чеком возврата) и только потом пишет реверс в журнал; неудачный вызов не пишет **ничего** — журнал не - может заявить о возврате, которого не было. Записывается собственный refund-id провайдера (сверка с + может заявить о возврате, которого не было. Реверс пишется **только при статусе `succeeded`**: + возврат в `pending` ещё может отмениться, а журнал только на добавление, поэтому ранняя запись + отобрала бы у покупателя Фишки за деньги, оставшиеся у нас; выход — нажать ещё раз, ключ + идемпотентности вернёт тот же возврат. Записывается собственный refund-id провайдера (сверка с данными ЮKassa). VK и TG Stars — как раньше: деньги руками, запись фактом. +- **D52. Обрабатываем `refund.succeeded`: кабинет — вторая точка входа для возврата.** В отличие от + прежних рельсов, у ЮKassa возврат можно оформить прямо в кабинете магазина, минуя наш API, — тогда + деньги ушли бы назад, а Фишки остались бы начисленными, и молча. Поэтому подписываемся на событие и + проводим возврат тем же движком: возврат перечитывается из API (тело уведомления — не доказательство, + как и у платежа), привязывается к заказу через записанный идентификатор платежа, идемпотентность по + `(провайдер, refund-id)` делает уведомление о возврате, уже записанном консолью, пустой операцией. + **Частичный возврат не записываем вовсе** — движок по устройству работает только с полной суммой + заказа, а непроизвольного способа решить, скольких Фишек стоит часть возврата, нет; вместо записи — + громкий лог для оператора. - **D51. Фискализация — «Чеки от ЮKassa», чек отправляем из кода.** Ревизия D41: у ЮKassa нет кабинетного «обобщённого чека», как у Robokassa, — чек регистрируется **только если запрос его несёт**, поэтому itemized-код, от которого отказались в D41, возвращается в объём. Каждый платёж и @@ -336,7 +348,7 @@ web+PWA / native Android+iOS через Capacitor). Владелец (самоз ведёт к пропускам. Текст — только для фиксации решённого и пояснений. Усилить feedback-память `prefer-interview-mode` после plan mode. -## Все развилки закрыты (D1-D51) +## Все развилки закрыты (D1-D52) Интервью завершено. Дальше — оформление документов и реализация по релизам. @@ -355,6 +367,10 @@ D47-D51 — консервация Robokassa с откатом по кредам у ЮKassa **есть** тестовый режим (отдельный тестовый магазин со своими креды и тестовыми картами), поэтому весь путь проверяется на тестовом контуре без реальных денег. +**Уточнение по возвратам (владелец, 2026-07-28).** D50 дополнена требованием статуса `succeeded`; +добавлена D52 — обработка `refund.succeeded`, потому что кабинет ЮKassa позволяет вернуть деньги +мимо нашего API. Обе правки вошли в тот же PR, что и переход на рельс. + ## План внедрения (черновик PLAN.md — «слоями») Владелец выбрал слоёную стратегию: сначала вся механика без реальных денег (обкатка diff --git a/docs/PAYMENTS_ru.md b/docs/PAYMENTS_ru.md index f843a64..b7603da 100644 --- a/docs/PAYMENTS_ru.md +++ b/docs/PAYMENTS_ru.md @@ -298,13 +298,28 @@ at-least-once + идемпотентный приём (дедуп по `telegram бота). **Возвраты.** ToS — **невозвратно**, пользователю возврат не предлагаем. Возвраты **инициирует -админ** (консоль E7): ни один рельс не шлёт непрошеный возврат. На direct-рельсе консоль делает -всю работу одним нажатием (D50): сперва вызывает refund-API ЮKassa (`POST /v3/refunds`, -`Idempotence-Key` — идентификатор заказа, с чеком возврата из §12) и записывает реверс только после -того, как деньги действительно ушли, — неудачный вызов не пишет **ничего**, поэтому журнал не может -заявить о возврате, которого не было. Записывается собственный refund-id провайдера: по нему журнал -сверяется с данными ЮKassa. Возвраты VK по-прежнему через поддержку, TG Stars — вызовом -`refundStarPayment`, оба фиксируются руками после факта. Все сходятся на одном движке — метод `Refund` +админ** (консоль E7). На direct-рельсе консоль делает всю работу одним нажатием (D50): сперва +вызывает refund-API ЮKassa (`POST /v3/refunds`, `Idempotence-Key` — идентификатор заказа, с чеком +возврата из §12) и записывает реверс только после того, как деньги действительно ушли, — неудачный +вызов не пишет **ничего**, поэтому журнал не может заявить о возврате, которого не было. Возврат +записывается **только в статусе `succeeded`**: ещё не завершённый (`pending`) может отмениться, а +журнал только на добавление, поэтому ранняя запись отобрала бы у покупателя Фишки за деньги, оставшиеся у +нас. Выход — нажать ещё раз: ключ идемпотентности вернёт тот же возврат, а не заплатит дважды. +Записывается собственный refund-id провайдера: по нему журнал сверяется с данными ЮKassa. + +**Кабинет — вторая точка входа (D52).** В отличие от прежних рельсов, ЮKassa позволяет оформить +возврат прямо в кабинете магазина, и такой возврат не проходит через наш API — деньги ушли бы назад, +а Фишки остались бы начисленными. Это закрывает уведомление `refund.succeeded`: возврат +перечитывается из API (тело уведомления — такое же не-доказательство, как и у платежа), привязывается +к заказу через идентификатор платежа, который мы записали, и проводится тем же движком — +идемпотентно по `(провайдер, refund-id)`, поэтому уведомление о возврате, уже записанном консолью, +ничего не отзывает повторно. Движок по устройству работает **только с полным возвратом** (отзывает +ровно то, что профондировал пакет, и отвергает любую другую сумму), поэтому **частичный** возврат не +записывается вовсе и громко логируется для оператора: непроизвольного способа решить, скольких Фишек +стоит часть возврата, нет. + +Возвраты VK по-прежнему через поддержку, TG Stars — вызовом `refundStarPayment`, оба фиксируются +руками после факта. Все сходятся на одном движке — метод `Refund` (`internal/payments`): матчит оплаченный заказ, пишет **refund**-строку журнала (идемпотентно по `(provider, provider_refund_id)` — refund-id отличается от payment-id fund'а, поэтому строки сосуществуют под тем же partial-unique индексом) и **по возможности отзывает начисленные Фишки с