From 05ee5b9d2e0a01ef37431f68363adcaf5d7c7f59 Mon Sep 17 00:00:00 2001 From: shanshanzhong Date: Mon, 25 May 2026 00:35:54 -0700 Subject: [PATCH] fix: P1 activation path hardening - Bug 4-9 Bug 4: resolveRenewalActivationSubscription - add fallback by user_id+subscribe_id with SELECT FOR UPDATE when token lookup fails Bug 5: appleIAPNotifyLogic - return error on product ID mapping failure instead of silently dropping the notification Bug 6: NewPurchase fallback query - wrap in transaction with SELECT FOR UPDATE to prevent concurrent duplicate subscription creation Bug 7: appleIAPNotifyLogic - fix UserId=0 by reverse-lookup from original purchase order; create renewal audit order record for DID_RENEW/SUBSCRIBED notifications Bug 8: UpdateOrderStatus - pre-delete cache before DB write (double-delete) to close TOCTOU window between DB update and cache invalidation Bug 9: validateNewUserOnlyEligibilityAtActivation - add Redis distributed lock on user_id to serialise concurrent new-user-only order activations Co-authored-by: multica-agent --- internal/logic/notify/appleIAPNotifyLogic.go | 87 +++++++++++++++++++- internal/model/order/model.go | 6 +- queue/logic/order/activateOrderLogic.go | 54 ++++++++++-- queue/logic/order/newUserEligibility.go | 16 ++++ 4 files changed, 153 insertions(+), 10 deletions(-) diff --git a/internal/logic/notify/appleIAPNotifyLogic.go b/internal/logic/notify/appleIAPNotifyLogic.go index ed2944b..afe6776 100644 --- a/internal/logic/notify/appleIAPNotifyLogic.go +++ b/internal/logic/notify/appleIAPNotifyLogic.go @@ -3,17 +3,20 @@ package notify import ( "context" "encoding/json" + "fmt" "strconv" "strings" commonLogic "github.com/perfect-panel/server/internal/logic/common" iapmodel "github.com/perfect-panel/server/internal/model/iap/apple" + "github.com/perfect-panel/server/internal/model/order" "github.com/perfect-panel/server/internal/model/subscribe" "github.com/perfect-panel/server/internal/model/user" "github.com/perfect-panel/server/internal/svc" "github.com/perfect-panel/server/internal/types" iapapple "github.com/perfect-panel/server/pkg/iap/apple" "github.com/perfect-panel/server/pkg/logger" + "github.com/perfect-panel/server/pkg/tool" "gorm.io/gorm" ) @@ -84,6 +87,7 @@ func (l *AppleIAPNotifyLogic) Handle(signedPayload string) error { l.Errorw("iap notify insert transaction error", logger.Field("error", e.Error()), logger.Field("productId", txPayload.ProductId), logger.Field("originalTransactionId", txPayload.OriginalTransactionId)) return e } + existing = rec } else { if txPayload.RevocationDate != nil { // 撤销场景：更新 revocation_at @@ -96,6 +100,32 @@ func (l *AppleIAPNotifyLogic) Handle(signedPayload string) error { } } } + + // Fix Bug 7: resolve userId when the IAP transaction record has no associated user. + // This can happen if the first notification for a subscription arrived before the + // in-app purchase flow created the order (race) or the order was made in a previous + // build that did not write user_id to the IAP transaction table. + if existing != nil && existing.UserId == 0 { + var origOrder order.Order + if lookupErr := db.Model(&order.Order{}). + Where("trade_no = ? AND method = ?", txPayload.OriginalTransactionId, "apple_iap"). + Order("id ASC"). + First(&origOrder).Error; lookupErr == nil && origOrder.UserId > 0 { + existing.UserId = origOrder.UserId + _ = db.Model(&iapmodel.Transaction{}). + Where("id = ?", existing.Id). + Update("user_id", origOrder.UserId).Error + l.Infow("iap notify resolved zero userId from original purchase order", + logger.Field("userId", origOrder.UserId), + logger.Field("originalTransactionId", txPayload.OriginalTransactionId), + ) + } else { + l.Errorw("CRITICAL: iap notify UserId=0 and cannot resolve from order, notification dropped", + logger.Field("originalTransactionId", txPayload.OriginalTransactionId)) + return fmt.Errorf("iap notify: UserId=0 and cannot resolve from order for original_transaction_id=%s", txPayload.OriginalTransactionId) + } + } + var days int64 { pid := strings.ToLower(txPayload.ProductId) @@ -169,7 +199,12 @@ func (l *AppleIAPNotifyLogic) Handle(signedPayload string) error { } } if days == 0 { - l.Errorw("iap notify product mapping missing", logger.Field("productId", txPayload.ProductId)) + // Both string-parse and DB fallback failed to map the product to days. + // Return an error so Apple retries the notification; silently ignoring + // this would cause the subscriber's renewal to be lost. + l.Errorw("CRITICAL: iap notify product mapping missing, returning error to trigger retry", + logger.Field("productId", txPayload.ProductId)) + return fmt.Errorf("iap product id %s could not be mapped to subscription days", txPayload.ProductId) } token := "iap:" + txPayload.OriginalTransactionId sub, e := l.svcCtx.UserModel.FindOneSubscribeByToken(l.ctx, token) @@ -216,6 +251,11 @@ func (l *AppleIAPNotifyLogic) Handle(signedPayload string) error { logger.Field("product_id", txPayload.ProductId), )..., ) + // Create audit order record for renewal notifications (Bug 7) + if err := l.createIAPRenewalAuditOrder(db, ntype, txPayload.TransactionId, candidate.UserId, candidate.SubscribeId, candidate.Token); err != nil { + l.Errorw("iap notify fallback create renewal order error", logger.Field("error", err.Error())) + // Non-fatal: subscription already updated; order creation failure is logged only + } break } } @@ -248,7 +288,52 @@ func (l *AppleIAPNotifyLogic) Handle(signedPayload string) error { logger.Field("product_id", txPayload.ProductId), )..., ) + // Create audit order record for renewal notifications (Bug 7) + if err := l.createIAPRenewalAuditOrder(db, ntype, txPayload.TransactionId, sub.UserId, sub.SubscribeId, sub.Token); err != nil { + l.Errorw("iap notify create renewal order error", logger.Field("error", err.Error())) + // Non-fatal: subscription already updated; order creation failure is logged only + } } return nil }) } + +// createIAPRenewalAuditOrder creates a finished renewal order record for DID_RENEW and SUBSCRIBED +// Apple SSNS notifications so that the order table has a complete financial audit trail. +// The operation is idempotent: if an order with the same trade_no already exists it is skipped. +func (l *AppleIAPNotifyLogic) createIAPRenewalAuditOrder(db *gorm.DB, ntype, transactionId string, userId, subscribeId int64, subscribeToken string) error { + if ntype != "DID_RENEW" && ntype != "SUBSCRIBED" { + return nil + } + // Idempotency check + var count int64 + if err := db.Model(&order.Order{}). + Where("trade_no = ? AND method = ?", transactionId, "apple_iap"). + Count(&count).Error; err != nil { + return fmt.Errorf("check existing iap renewal order: %w", err) + } + if count > 0 { + return nil + } + rec := &order.Order{ + UserId: userId, + OrderNo: tool.GenerateTradeNo(), + Type: 2, // OrderTypeRenewal + Status: 5, // OrderStatusFinished + Method: "apple_iap", + TradeNo: transactionId, + SubscribeId: subscribeId, + SubscribeToken: subscribeToken, + Quantity: 1, + IsNew: false, + } + if err := db.Model(&order.Order{}).Create(rec).Error; err != nil { + return fmt.Errorf("create iap renewal audit order: %w", err) + } + l.Infow("iap notify created renewal audit order", + logger.Field("orderNo", rec.OrderNo), + logger.Field("transactionId", transactionId), + logger.Field("userId", userId), + ) + return nil +} diff --git a/internal/model/order/model.go b/internal/model/order/model.go index e175b19..89d1b5f 100644 --- a/internal/model/order/model.go +++ b/internal/model/order/model.go @@ -110,6 +110,10 @@ func (m *customOrderModel) UpdateOrderStatus(ctx context.Context, orderNo string if err != nil { return err } + keys := m.getCacheKeys(orderInfo) + // Pre-delete: evict cache before the DB write so concurrent reads during the update + // window go to DB instead of getting a stale cached status (double-delete pattern). + _ = m.DelCacheCtx(ctx, keys...) return m.ExecCtx(ctx, func(conn *gorm.DB) error { if len(tx) > 0 { conn = tx[0] @@ -123,7 +127,7 @@ func (m *customOrderModel) UpdateOrderStatus(ctx context.Context, orderNo string return nil } return nil - }, m.getCacheKeys(orderInfo)...) + }, keys...) } // FindOneDetailsByOrderNo Find order details by order number diff --git a/queue/logic/order/activateOrderLogic.go b/queue/logic/order/activateOrderLogic.go index ef0e24c..4f7eaba 100644 --- a/queue/logic/order/activateOrderLogic.go +++ b/queue/logic/order/activateOrderLogic.go @@ -597,7 +597,7 @@ func (l *ActivateOrderLogic) NewPurchase(ctx context.Context, orderInfo *order.O return err } - if err = validateNewUserOnlyEligibilityAtActivation(ctx, l.svc.DB, orderInfo, sub); err != nil { + if err = validateNewUserOnlyEligibilityAtActivation(ctx, l.svc.DB, l.svc.Redis, orderInfo, sub); err != nil { return err } @@ -675,18 +675,23 @@ func (l *ActivateOrderLogic) NewPurchase(ctx context.Context, orderInfo *order.O // 兜底：创建新订阅前，查找用户是否已有同套餐的订阅记录（含过期/赠送）， // 有则复用旧记录续期，避免出现重复订阅。 // 需要同时检查 UserId 和 SubscriptionUserId，因为家庭组绑定前后 owner 可能不同。 + // 使用 SELECT ... FOR UPDATE 防止并发下多个 worker 同时选中同一订阅并创建重复记录。 if userSub == nil { candidateUserIds := []int64{orderInfo.UserId} if orderInfo.SubscriptionUserId > 0 && orderInfo.SubscriptionUserId != orderInfo.UserId { candidateUserIds = append(candidateUserIds, orderInfo.SubscriptionUserId) } var existingSub user.Subscribe - if findErr := l.svc.DB.Model(&user.Subscribe{}). - Where("user_id IN ? AND token != ''", candidateUserIds). - Order("expire_time DESC"). - Order("updated_at DESC"). - Order("id DESC"). - First(&existingSub).Error; findErr == nil { + findErr := l.svc.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Model(&user.Subscribe{}). + Where("user_id IN ? AND token != ''", candidateUserIds). + Order("expire_time DESC"). + Order("updated_at DESC"). + Order("id DESC"). + First(&existingSub).Error + }) + if findErr == nil { // 家庭组场景：订阅 owner 可能变更（如成员注册的试用 → 被家主收归）， // 续期前把 user_id 校正为当前订单的 SubscriptionUserId effectiveOwner := orderInfo.UserId @@ -1447,7 +1452,40 @@ func (l *ActivateOrderLogic) resolveRenewalActivationSubscription(ctx context.Co } userSub, err := l.getUserSubscription(ctx, orderInfo.SubscribeToken) if err != nil { - return nil, err + // Fallback: token may have been lost (subscription deleted or owner changed). + // Locate the most recent subscription by user_id + subscribe_id with FOR UPDATE + // to prevent a concurrent worker from renewing the same record twice. + targetUserID := orderInfo.UserId + if orderInfo.SubscriptionUserId > 0 { + targetUserID = orderInfo.SubscriptionUserId + } + var fallbackSub user.Subscribe + txErr := l.svc.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Model(&user.Subscribe{}). + Where("user_id = ? AND subscribe_id = ?", targetUserID, orderInfo.SubscribeId). + Where("status IN ?", []int64{0, 1, 2, 3}). + Order("expire_time DESC"). + Order("updated_at DESC"). + Order("id DESC"). + First(&fallbackSub).Error + }) + if txErr != nil { + logger.WithContext(ctx).Error("CRITICAL: Renewal activation token and fallback lookup both failed", + logger.Field("token_error", err.Error()), + logger.Field("fallback_error", txErr.Error()), + logger.Field("order_no", orderInfo.OrderNo), + logger.Field("target_user_id", targetUserID), + logger.Field("subscribe_id", orderInfo.SubscribeId), + ) + return nil, fmt.Errorf("renewal activation subscription not found by token or user_id+subscribe_id for order %s: %w", orderInfo.OrderNo, err) + } + logger.WithContext(ctx).Info("Renewal token lookup failed; found subscription via fallback user_id+subscribe_id", + logger.Field("order_no", orderInfo.OrderNo), + logger.Field("fallback_subscribe_id", fallbackSub.Id), + logger.Field("target_user_id", targetUserID), + ) + userSub = &fallbackSub } if orderInfo.UserId <= 0 { return userSub, nil diff --git a/queue/logic/order/newUserEligibility.go b/queue/logic/order/newUserEligibility.go index 76e6b61..79e7d6c 100644 --- a/queue/logic/order/newUserEligibility.go +++ b/queue/logic/order/newUserEligibility.go @@ -10,12 +10,14 @@ import ( "github.com/perfect-panel/server/internal/model/order" "github.com/perfect-panel/server/internal/model/subscribe" internaltypes "github.com/perfect-panel/server/internal/types" + "github.com/redis/go-redis/v9" "gorm.io/gorm" ) func validateNewUserOnlyEligibilityAtActivation( ctx context.Context, db *gorm.DB, + rdb *redis.Client, orderInfo *order.Order, sub *subscribe.Subscribe, ) error { @@ -31,6 +33,20 @@ func validateNewUserOnlyEligibilityAtActivation( return nil } + // Acquire a per-user distributed lock so concurrent new-user-only activations + // for the same account are serialised. Without this, two workers can both read + // historyCount=0 and both pass the check before either has written the order. + lockKey := fmt.Sprintf("new_user_only_activate:%d", orderInfo.UserId) + const lockTTL = 30 * time.Second + acquired, lockErr := rdb.SetNX(ctx, lockKey, orderInfo.OrderNo, lockTTL).Result() + if lockErr != nil { + return fmt.Errorf("new user only: acquire lock error: %w", lockErr) + } + if !acquired { + return fmt.Errorf("new user only: another activation is in progress for user %d", orderInfo.UserId) + } + defer rdb.Del(ctx, lockKey) + eligibility, err := commonLogic.ResolveNewUserEligibility(ctx, db, orderInfo.UserId) if err != nil { return err