Stripe checkout add customer full address details during checkout - #1824
Closed
sanjibnarzary wants to merge 6 commits into
Closed
Stripe checkout add customer full address details during checkout#1824sanjibnarzary wants to merge 6 commits into
sanjibnarzary wants to merge 6 commits into
Conversation
…untries where address is necessary
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughAdds Creem payment integration end-to-end: new settings, status flags, API endpoints (initiate pay + webhook), Creem checkout generation and webhook handling, Creem-specific recharge logic, admin UI for Creem products, TopUp UI flow changes, and Stripe checkout param/URL updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Frontend (TopUp)
participant API as API Server
participant CREEM as Creem API
participant DB as DB
U->>FE: Select Creem product
FE->>API: POST /api/user/creem/pay {productId, payment_method:"creem"}
API->>DB: Create TopUp (Pending)
API->>CREEM: genCreemLink (create checkout)
CREEM-->>API: {checkout_url, order_id}
API-->>FE: {checkout_url, order_id}
FE->>U: Open checkout_url (new tab)
sequenceDiagram
autonumber
participant CREEM as Creem Webhook
participant API as API Server
participant DB as DB
note over CREEM,API: Event: checkout.completed
CREEM->>API: POST /api/creem/webhook (signature header)
API->>API: Verify signature (if configured)
API->>DB: Lookup TopUp by reference_id FOR UPDATE
API->>DB: Mark TopUp Completed & Success, update user quota, set email if empty
API-->>CREEM: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (16)
setting/payment_creem.go (1)
3-6: Avoid plain exported globals for secrets; add validation and typed products.
- Secrets as package-level vars are easy to leak and hard to rotate; at minimum, validate they’re non-empty at startup and avoid logging them. Consider getters that read from a config store with RLock.
- CreemProducts as a JSON string invites repeated unmarshal and runtime errors. Prefer a typed slice and marshal only at API boundaries.
Apply this minimal improvement to reduce runtime JSON handling elsewhere:
-var CreemProducts = "[]" +// JSON string for backward compatibility; prefer using a typed accessor. +var CreemProducts = "[]" + +// Helper: safely decode products once where needed. +func GetCreemProducts() ([]struct{ + ProductId string `json:"productId"` + Name string `json:"name"` + Price float64 `json:"price"` + Currency string `json:"currency"` + Quota int64 `json:"quota"` +}, error) { + var ps []struct{ + ProductId string `json:"productId"` + Name string `json:"name"` + Price float64 `json:"price"` + Currency string `json:"currency"` + Quota int64 `json:"quota"` + } + if err := json.Unmarshal([]byte(CreemProducts), &ps); err != nil { + return nil, err + } + return ps, nil +}controller/topup_stripe.go (1)
262-275: Prefer native phone_number_collection and persist address to Customer.
- Using CustomFields for phone yields unvalidated free text and isn’t surfaced as Customer.phone. Use PhoneNumberCollection.
- Ensure collected billing/shipping addresses persist to the Customer for future use.
Apply:
- BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)), - CustomFields: []*stripe.CheckoutSessionCustomFieldParams{ - { - Key: stripe.String("customer_phone"), - Label: &stripe.CheckoutSessionCustomFieldLabelParams{ - Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)), - Custom: stripe.String("Phone Number"), - }, - Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)), - Optional: stripe.Bool(false), - }, - }, + BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)), + PhoneNumberCollection: &stripe.CheckoutSessionPhoneNumberCollectionParams{ + Enabled: stripe.Bool(true), + }, + CustomerUpdate: &stripe.CheckoutSessionCustomerUpdateParams{ + Address: stripe.String(string(stripe.CheckoutSessionCustomerUpdateAddressAuto)), + Shipping: stripe.String(string(stripe.CheckoutSessionCustomerUpdateShippingAuto)), + },controller/topup_creem.go (2)
352-355: Sanitize PII in logs.Avoid logging full email/name; mask or omit.
Apply:
-log.Printf("Creem充值成功 - 订单号: %s, 充值额度: %d, 支付金额: %.2f, 客户邮箱: %s, 客户姓名: %s", - referenceId, topUp.Amount, topUp.Money, customerEmail, customerName) +log.Printf("Creem充值成功 - 订单号: %s, 充值额度: %d, 支付金额: %.2f", + referenceId, topUp.Amount, topUp.Money)
437-441: Be tolerant to 201 Created; many APIs return 201 for creates.Strict 200 check may reject valid responses.
Apply:
-if resp.StatusCode != http.StatusOK { +if resp.StatusCode/100 != 2 { return "", fmt.Errorf("Creem API 返回错误状态 %d: %s", resp.StatusCode, string(body)) }controller/misc.go (1)
77-79: Gate Creem top‑up more robustly; return typed products.
- Checking
CreemProducts != "[]"is brittle (whitespace/order). Parse and ensure at least one valid product.- Consider requiring
CreemWebhookSecrettoo so funds aren’t accepted without a secure crediting path.- Returning products as a JSON string forces clients to reparse.
Apply:
- "enable_creem_topup": setting.CreemApiKey != "" && setting.CreemProducts != "[]", - "creem_products": setting.CreemProducts, + "enable_creem_topup": func() bool { + ps, err := setting.GetCreemProducts() + return setting.CreemApiKey != "" && err == nil && len(ps) > 0 && setting.CreemWebhookSecret != "" + }(), + "creem_products": func() []any { + ps, err := setting.GetCreemProducts() + if err != nil { return []any{} } + // return as typed slice; frontend no longer needs to JSON.parse + out := make([]any, 0, len(ps)) + for _, p := range ps { out = append(out, p) } + return out + }(),web/src/components/settings/PaymentSetting.js (2)
30-33: Include CreemTestMode in the initial state for stable props shapePrevents undefined on first render before options load.
StripeMinTopUp: 1, - - CreemApiKey: '', - CreemWebhookSecret: '', - CreemProducts: '[]', + CreemApiKey: '', + CreemWebhookSecret: '', + CreemProducts: '[]', + CreemTestMode: 'false',
52-59: Remove no-op try/catch when copying CreemProductsYou’re not parsing here; the try/catch is dead code.
- case 'CreemProducts': - try { - newInputs[item.key] = item.value; - } catch (error) { - console.error('解析CreemProducts出错:', error); - newInputs[item.key] = '[]'; - } - break; + case 'CreemProducts': + newInputs[item.key] = item.value || '[]'; + break;model/option.go (1)
333-340: Validate CreemProducts JSON before persistingA malformed JSON here can break frontend parsing and status exposure. Reject or sanitize invalid payloads.
- case "CreemProducts": - setting.CreemProducts = value + case "CreemProducts": + if !isValidCreemProductsJSON(value) { + return fmt.Errorf("invalid CreemProducts JSON") + } + setting.CreemProducts = valueAdd helper (outside this hunk):
// at top: import ( "encoding/json" "fmt" // ... ) type creemProduct struct { Name string `json:"name"` ProductId string `json:"productId"` Price float64 `json:"price"` Quota int64 `json:"quota"` Currency string `json:"currency"` } func isValidCreemProductsJSON(s string) bool { if s == "" { return true } var arr []creemProduct if err := json.Unmarshal([]byte(s), &arr); err != nil { return false } for _, p := range arr { if p.Name == "" || p.ProductId == "" || p.Price <= 0 || p.Quota <= 0 { return false } } return true }web/src/pages/TopUp/index.js (4)
313-341: Handle backend error shape defensivelyIf backend returns {success:false,message:'error',data:'...'}, current branches are fine. Consider surfacing non‑string data robustly, but optional.
- } else { - showError(data); + } else { + showError(typeof data === 'string' ? data : t('请求失败')); }
343-346: Prevent reverse‑tabnabbing when opening checkout URLUse noopener,noreferrer.
- window.open(data.checkout_url, '_blank'); + window.open(data.checkout_url, '_blank', 'noopener,noreferrer');
689-714: Price display: format to two decimalsCleaner UI for currency.
- {t('价格')}：{selectedCreemProduct.currency === 'EUR' ? '€' : '$'}{selectedCreemProduct.price} + {t('价格')}：{selectedCreemProduct.currency === 'EUR' ? '€' : '$'}{Number(selectedCreemProduct.price).toFixed(2)}
1196-1213: Use stable keys in listsUse productId instead of the array index to avoid reconciliation issues.
- {creemProducts.map((product, index) => ( - <Card key={index} + {creemProducts.map((product) => ( + <Card key={product.productId}Apply to both desktop and mobile maps.
Also applies to: 1231-1248
web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js (4)
78-85: Allow clearing secretsCurrent logic won’t persist clearing API/Webhook secrets. Always send the keys.
- if (inputs.CreemApiKey && inputs.CreemApiKey !== '') { - options.push({ key: 'CreemApiKey', value: inputs.CreemApiKey }); - } + options.push({ key: 'CreemApiKey', value: inputs.CreemApiKey || '' }); - if (inputs.CreemWebhookSecret && inputs.CreemWebhookSecret !== '') { - options.push({ key: 'CreemWebhookSecret', value: inputs.CreemWebhookSecret }); - } + options.push({ key: 'CreemWebhookSecret', value: inputs.CreemWebhookSecret || '' });
149-153: Trim inputs and enforce types in validationPrevents subtle errors from whitespace or non‑numeric values.
- if (!productForm.name || !productForm.productId || productForm.price <= 0 || productForm.quota <= 0 || !productForm.currency) { + const name = productForm.name.trim(); + const productId = productForm.productId.trim(); + const price = Number(productForm.price); + const quota = Number(productForm.quota); + if (!name || !productId || !isFinite(price) || price <= 0 || !Number.isInteger(quota) || quota <= 0 || !productForm.currency) { showError(t('请填写完整的产品信息')); return; }Also assign trimmed values on save:
- newProducts[index] = { ...productForm }; + newProducts[index] = { ...productForm, name, productId, price, quota };
195-195: Format price in tableDisplay as currency with two decimals.
- render: (price, record) => `${record.currency === 'EUR' ? '€' : '$'}${price}`, + render: (price, record) => `${record.currency === 'EUR' ? '€' : '$'}${Number(price).toFixed(2)}`,
175-178: Confirm before deleting a productAvoid accidental removals.
-const deleteProduct = (productId) => { - const newProducts = products.filter(p => p.productId !== productId); - setProducts(newProducts); -}; +const deleteProduct = (productId) => { + Modal.confirm({ + title: t('确认删除？'), + content: t('删除后需重新保存设置才会生效'), + onOk: () => { + setProducts(prev => prev.filter(p => p.productId !== productId)); + }, + }); +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
controller/misc.go(1 hunks)controller/topup_creem.go(1 hunks)controller/topup_stripe.go(1 hunks)model/option.go(2 hunks)model/topup.go(1 hunks)router/api-router.go(2 hunks)setting/payment_creem.go(1 hunks)web/src/components/settings/PaymentSetting.js(4 hunks)web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js(1 hunks)web/src/pages/TopUp/index.js(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
controller/misc.go (1)
setting/payment_creem.go (2)
CreemApiKey(3-3)CreemProducts(4-4)
model/option.go (2)
common/constants.go (1)
OptionMap(36-36)setting/payment_creem.go (4)
CreemApiKey(3-3)CreemProducts(4-4)CreemTestMode(5-5)CreemWebhookSecret(6-6)
web/src/pages/TopUp/index.js (4)
web/src/helpers/utils.js (1)
showError(94-123)web/src/helpers/api.js (6)
res(186-186)res(187-187)res(228-228)res(229-229)API(5-13)API(5-13)web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js (1)
products(35-35)web/src/helpers/render.js (1)
renderQuotaWithAmount(856-864)
web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js (2)
web/src/components/settings/PaymentSetting.js (1)
inputs(13-33)web/src/helpers/utils.js (3)
showError(94-123)showSuccess(129-131)a(231-231)
router/api-router.go (2)
controller/topup_creem.go (2)
CreemWebhook(237-284)RequestCreemPay(140-164)middleware/rate-limit.go (1)
CriticalRateLimit(103-105)
controller/topup_stripe.go (2)
setting/system_setting.go (1)
ServerAddress(3-3)setting/payment_stripe.go (1)
StripePriceId(5-5)
model/topup.go (7)
common/database.go (1)
UsingPostgreSQL(10-10)model/main.go (1)
DB(63-63)common/constants.go (2)
TopUpStatusPending(198-198)TopUpStatusSuccess(199-199)common/utils.go (1)
GetTimestamp(192-194)model/user.go (1)
User(18-47)model/log.go (2)
RecordLog(76-92)LogTypeTopup(41-41)common/logger.go (1)
FormatQuota(107-113)
controller/topup_creem.go (4)
setting/payment_creem.go (4)
CreemProducts(4-4)CreemWebhookSecret(6-6)CreemApiKey(3-3)CreemTestMode(5-5)model/user.go (1)
GetUserById(224-236)common/hash.go (1)
Sha1(22-24)model/topup.go (3)
TopUp(11-20)GetTopUpByTradeNo(44-52)RechargeCreem(102-171)
web/src/components/settings/PaymentSetting.js (1)
web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js (2)
SettingsPaymentGatewayCreem(25-387)inputs(28-33)
🔇 Additional comments (9)
controller/topup_creem.go (1)
120-137: Order creation before external call is good; consider idempotency key to Creem.If the POST is retried, upstream may create duplicates. If Creem supports idempotency keys, send referenceId as such.
Would you confirm whether Creem’s API supports an Idempotency-Key header and, if so, reuse referenceId for it?
router/api-router.go (1)
68-68: Route parity looks goodUser-auth + CriticalRateLimit mirrors the Stripe path. No issues.
web/src/components/settings/PaymentSetting.js (2)
6-6: Import addition is fineCreem settings component is properly wired.
109-111: Rendering the Creem settings card is OKIntegration point is consistent with the other gateways.
model/option.go (1)
84-87: Options exposure LGTMCreem keys added to OptionMap align with setting/payment_creem.go.
web/src/pages/TopUp/index.js (3)
69-73: State wiring for Creem looks correctNo issues with the added state.
304-311: Pre‑topup UX is fineOpens confirmation modal with selected product; OK.
1017-1017: Banner gating logic reads wellWarns only when all top-up methods are disabled. OK.
web/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.js (1)
47-67: Initialization flow is soundForm prefill and product parsing behave as expected.
Comment on lines
+37
to
+45
| func verifyCreemSignature(payload string, signature string, secret string) bool { | ||
| if secret == "" { | ||
| log.Printf("Creem webhook secret未配置，跳过签名验证") | ||
| return true // 如果没有配置secret，跳过验证 | ||
| } | ||
|
|
||
| expectedSignature := generateCreemSignature(payload, secret) | ||
| return hmac.Equal([]byte(signature), []byte(expectedSignature)) | ||
| } |
Contributor
There was a problem hiding this comment.
Webhook signature without timestamp is replayable; also skip‑verify on empty secret.
- Consider including a timestamp header in the MAC (e.g., HMAC of ts + “.” + payload) and rejecting stale timestamps.
- Don’t accept webhooks when secret is unset; fail closed.
Apply:
-func verifyCreemSignature(payload string, signature string, secret string) bool {
- if secret == "" {
- log.Printf("Creem webhook secret未配置，跳过签名验证")
- return true
- }
+func verifyCreemSignature(payload string, signature string, secret string, ts string) bool {
+ if secret == "" {
+ log.Printf("Creem webhook secret未配置")
+ return false
+ }
- expectedSignature := generateCreemSignature(payload, secret)
+ signed := ts + "." + payload
+ expectedSignature := generateCreemSignature(signed, secret)
return hmac.Equal([]byte(signature), []byte(expectedSignature))
}And in handler, require a recent timestamp (e.g., 5 min) before comparing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func verifyCreemSignature(payload string, signature string, secret string) bool { | |
| if secret == "" { | |
| log.Printf("Creem webhook secret未配置，跳过签名验证") | |
| return true // 如果没有配置secret，跳过验证 | |
| } | |
| expectedSignature := generateCreemSignature(payload, secret) | |
| return hmac.Equal([]byte(signature), []byte(expectedSignature)) | |
| } | |
| func verifyCreemSignature(payload string, signature string, secret string, ts string) bool { | |
| if secret == "" { | |
| log.Printf("Creem webhook secret未配置") | |
| return false | |
| } | |
| signed := ts + "." + payload | |
| expectedSignature := generateCreemSignature(signed, secret) | |
| return hmac.Equal([]byte(signature), []byte(expectedSignature)) | |
| } |
🤖 Prompt for AI Agents
In controller/topup_creem.go around lines 37-45, the current
verifyCreemSignature skips verification when secret is empty and uses a
signature over only the payload (replayable); change to fail-closed and include
a timestamp in the MAC: require the request to supply a timestamp header (e.g.,
"Creem-Timestamp"), reject if missing or older than 5 minutes, compute
expectedSignature = HMAC(secret, timestamp + "." + payload) and compare with the
provided signature using hmac.Equal, and if secret is empty return false (log an
error) instead of skipping verification; also update the handler to parse the
timestamp header and enforce the freshness check before calling the HMAC
comparison.
Comment on lines
+97
to
+103
| id := c.GetInt("id") | ||
| user, _ := model.GetUserById(id, false) | ||
|
|
||
| // 生成唯一的订单引用ID | ||
| reference := fmt.Sprintf("creem-api-ref-%d-%d-%s", user.Id, time.Now().UnixMilli(), randstr.String(4)) | ||
| referenceId := "ref_" + common.Sha1([]byte(reference)) | ||
|
|
Contributor
There was a problem hiding this comment.
Nil deref risk: unchecked GetUserById error.
If the user lookup fails, dereferencing user will panic.
Apply:
- user, _ := model.GetUserById(id, false)
+ user, err := model.GetUserById(id, false)
+ if err != nil || user == nil {
+ log.Printf("获取用户失败: id=%d, err=%v", id, err)
+ c.JSON(200, gin.H{"message": "error", "data": "用户不存在"})
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| id := c.GetInt("id") | |
| user, _ := model.GetUserById(id, false) | |
| // 生成唯一的订单引用ID | |
| reference := fmt.Sprintf("creem-api-ref-%d-%d-%s", user.Id, time.Now().UnixMilli(), randstr.String(4)) | |
| referenceId := "ref_" + common.Sha1([]byte(reference)) | |
| id := c.GetInt("id") | |
| user, err := model.GetUserById(id, false) | |
| if err != nil || user == nil { | |
| log.Printf("获取用户失败: id=%d, err=%v", id, err) | |
| c.JSON(200, gin.H{"message": "error", "data": "用户不存在"}) | |
| return | |
| } | |
| // 生成唯一的订单参考ID | |
| reference := fmt.Sprintf("creem-api-ref-%d-%d-%s", user.Id, time.Now().UnixMilli(), randstr.String(4)) | |
| referenceId := "ref_" + common.Sha1([]byte(reference)) |
🤖 Prompt for AI Agents
In controller/topup_creem.go around lines 97 to 103, the call to
model.GetUserById ignores its error and immediately dereferences user, which can
cause a nil-pointer panic when the lookup fails; update the code to check the
returned error (and whether user is nil), handle the failure path (e.g., return
an HTTP error/abort the request or log and return), and only proceed to build
the reference and use user.Id after confirming user is non-nil and err is nil.
Comment on lines
+143
to
+159
| // 读取body内容用于打印，同时保留原始数据供后续使用 | ||
| bodyBytes, err := io.ReadAll(c.Request.Body) | ||
| if err != nil { | ||
| log.Printf("读取请求body失败: %v", err) | ||
| c.JSON(200, gin.H{"message": "error", "data": "读取请求失败"}) | ||
| return | ||
| } | ||
|
|
||
| // 打印body内容 | ||
| log.Printf("creem pay request body: %s", string(bodyBytes)) | ||
|
|
||
| // 重新设置body供后续的ShouldBindJSON使用 | ||
| c.Request.Body = io.NopCloser(bytes.NewReader(bodyBytes)) | ||
|
|
||
| err = c.ShouldBindJSON(&req) | ||
| log.Printf(" json body is %+v", req) | ||
| if err != nil { |
Contributor
There was a problem hiding this comment.
Avoid logging raw request bodies (PII).
Body can contain identifiers; log minimal metadata or gate behind debug.
Apply:
-// 打印body内容
-log.Printf("creem pay request body: %s", string(bodyBytes))
+// Debug only: redact in production
+if common.DebugEnabled {
+ log.Printf("creem pay request received (%d bytes)", len(bodyBytes))
+}
@@
-log.Printf(" json body is %+v", req)
+if common.DebugEnabled {
+ log.Printf("creem pay parsed: product_id=%s", req.ProductId)
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In controller/topup_creem.go around lines 143 to 159 the code reads and logs the
entire request body (potential PII); instead of logging the raw body, remove the
raw body log and either log only minimal, non-sensitive metadata (e.g., content
length, request ID, payment provider, or a debug-only flag) or pass the body
through a redaction function that strips identifiers before logging; keep the
steps that read and restore c.Request.Body for ShouldBindJSON, and if you add a
debug gate, use the existing logger or a configuration flag so full bodies are
only emitted when explicitly enabled.
Comment on lines
+246
to
+260
| // 获取签名头 | ||
| signature := c.GetHeader(CreemSignatureHeader) | ||
|
|
||
| // 打印请求信息用于调试 | ||
| log.Printf("Creem Webhook - URI: %s, Query: %s", c.Request.RequestURI, c.Request.URL.RawQuery) | ||
| log.Printf("Creem Webhook - Signature: %s", signature) | ||
| log.Printf("Creem Webhook - Body: %s", string(bodyBytes)) | ||
|
|
||
| // 验证签名 | ||
| if !verifyCreemSignature(string(bodyBytes), signature, setting.CreemWebhookSecret) { | ||
| log.Printf("Creem Webhook签名验证失败") | ||
| c.AbortWithStatus(http.StatusUnauthorized) | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Fail closed on missing/invalid webhook signature; capture timestamp header.
Currently, missing secret still passes verification, and no anti‑replay check is in place.
Apply:
- signature := c.GetHeader(CreemSignatureHeader)
+ signature := c.GetHeader(CreemSignatureHeader)
+ ts := c.GetHeader("creem-timestamp")
@@
- if !verifyCreemSignature(string(bodyBytes), signature, setting.CreemWebhookSecret) {
+ if !verifyCreemSignature(string(bodyBytes), signature, setting.CreemWebhookSecret, ts) {
log.Printf("Creem Webhook签名验证失败")
c.AbortWithStatus(http.StatusUnauthorized)
return
}Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+232
to
+261
| ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{ | ||
| AllowedCountries: stripe.StringSlice([]string{ | ||
| "AC", "AD", "AE", "AF", "AG", "AI", "AL", "AM", "AO", "AQ", "AR", "AT", "AU", "AW", "AX", "AZ", | ||
| "BA", "BB", "BD", "BE", "BF", "BG", "BH", "BI", "BJ", "BL", "BM", "BN", "BO", "BQ", "BR", "BS", "BT", "BV", "BW", "BY", "BZ", | ||
| "CA", "CD", "CF", "CG", "CH", "CI", "CK", "CL", "CM", "CN", "CO", "CR", "CV", "CW", "CY", "CZ", | ||
| "DE", "DJ", "DK", "DM", "DO", "DZ", | ||
| "EC", "EE", "EG", "EH", "ER", "ES", "ET", | ||
| "FI", "FJ", "FK", "FO", "FR", | ||
| "GA", "GB", "GD", "GE", "GF", "GG", "GH", "GI", "GL", "GM", "GN", "GP", "GQ", "GR", "GS", "GT", "GU", "GW", "GY", | ||
| "HK", "HN", "HR", "HT", "HU", | ||
| "ID", "IE", "IL", "IM", "IN", "IO", "IQ", "IS", "IT", | ||
| "JE", "JM", "JO", "JP", | ||
| "KE", "KG", "KH", "KI", "KM", "KN", "KR", "KW", "KY", "KZ", | ||
| "LA", "LB", "LC", "LI", "LK", "LR", "LS", "LT", "LU", "LV", "LY", | ||
| "MA", "MC", "MD", "ME", "MF", "MG", "MK", "ML", "MM", "MN", "MO", "MQ", "MR", "MS", "MT", "MU", "MV", "MW", "MX", "MY", "MZ", | ||
| "NA", "NC", "NE", "NG", "NI", "NL", "NO", "NP", "NR", "NU", "NZ", | ||
| "OM", | ||
| "PA", "PE", "PF", "PG", "PH", "PK", "PL", "PM", "PN", "PR", "PS", "PT", "PY", | ||
| "QA", | ||
| "RE", "RO", "RS", "RU", "RW", | ||
| "SA", "SB", "SC", "SD", "SE", "SG", "SH", "SI", "SJ", "SK", "SL", "SM", "SN", "SO", "SR", "SS", "ST", "SV", "SX", "SZ", | ||
| "TA", "TC", "TD", "TF", "TG", "TH", "TJ", "TK", "TL", "TM", "TN", "TO", "TR", "TT", "TV", "TW", "TZ", | ||
| "UA", "UG", "US", "UY", "UZ", | ||
| "VA", "VC", "VE", "VG", "VN", "VU", | ||
| "WF", "WS", | ||
| "XK", | ||
| "YE", "YT", | ||
| "ZA", "ZM", "ZW", | ||
| }), | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
AllowedCountries list likely contains unsupported codes; risk of Checkout creation failures.
Entries such as AC, TA, XK, BV, SJ, TF, GS are not standard ISO 3166-1 alpha-2 country codes accepted by Stripe shipping address collection. Recommend limiting to required markets (e.g., "IN") or verifying the official allowed set for Checkout sessions.
Please confirm the valid values for CheckoutSessionShippingAddressCollectionParams.AllowedCountries:
If the goal is India compliance, consider the minimal set:
- AllowedCountries: stripe.StringSlice([]string{ ...huge list... }),
+ AllowedCountries: stripe.StringSlice([]string{"IN"}),🌐 Web query:
Stripe Checkout shipping_address_collection.allowed_countries valid values for API v81; list of accepted country codes and whether AC, TA, XK, SJ, BV are supported.
💡 Result:
Short answer
- Stripe Checkout accepts two‑letter ISO 3166‑1 alpha‑2 country codes for shipping_address_collection.allowed_countries. [1][2]
- From Stripe/SDK docs, the historically unsupported codes include: AS, CX, CC, CU, HM, IR, KP, MH, FM, NF, MP, PW, SY, UM, VI. (So absence from that list ≠ guaranteed support — Stripe may change enums via changelog.) [3][4]
Your specific codes (API v81 / Checkout):
- AC — Not an ISO 3166‑1 alpha‑2 country code (so not supported for allowed_countries). [1][5]
- TA — Not an ISO 3166‑1 alpha‑2 country code (so not supported). [1][5]
- XK — XK is a user‑assigned / unofficial ISO code for Kosovo. Stripe has had inconsistent platform support (e.g., an Android SDK issue); XK may work in some contexts but is not an official ISO assignment and thus not guaranteed. If you need to accept Kosovo, test it in your target Stripe SDK/platform or contact Stripe support. [6][7]
- SJ — SJ (Svalbard & Jan Mayen) is an ISO alpha‑2 code and is allowed unless specifically blocked by a Stripe SDK/enumeration; Stripe uses ISO codes for allowed_countries. (No Stripe doc lists SJ as blocked.) [1][3]
- BV — BV (Bouvet Island) is an ISO alpha‑2 code and is allowed unless specifically blocked by Stripe (no Stripe doc lists BV as blocked). [1][3]
Sources
- Stripe docs: Checkout shipping_address_collection accepts two‑letter ISO country codes. [Collect shipping addresses / Checkout]. https://docs.stripe.com/payments/collect-addresses. [2]
- Stripe Checkout docs describing allowed_countries usage. https://docs.stripe.com/checkout/custom-checkout/collect-shipping-or-billing-information. [1]
- SDK / library notes listing historically unsupported country codes (shows which codes Stripe implementations have excluded). (example: Rust/Java SDK pages). [3][4]
- Stripe changelog example (adds/removes codes can change via changelog). (example: Sudan added 2025-01-27). https://docs.stripe.com/changelog. [5]
- ISO / country‑code references: AC/TA are not ISO 3166‑1 alpha‑2 assignments (they are not standard ISO country codes; Ascension/Tristan are grouped under SH). (See ISO usage and common references). [external ISO references].
- Stripe GitHub issue showing inconsistent XK/Kosovo support across SDKs. [BUG] Stripe Android SDK does not support Kosovo on billing information stripe/stripe-android#6501. [6]
- Notes on user‑assigned code XK (Kosovo) usage in practice. (e.g. Wikipedia / standards discussion). [7]
If you want, I can: (a) test creating a Checkout Session with each code and report whether the Stripe API v81 accepts/rejects them in real time, or (b) open a support request text you can send to Stripe to confirm official support.
Remove invalid/unofficial country codes from AllowedCountries and restrict to required markets (e.g., "IN").
- Location: controller/topup_stripe.go Lines 232–261.
- AC, TA — not ISO 3166‑1 alpha‑2 and will be rejected; remove.
- XK — user‑assigned/unofficial (Kosovo); Stripe support is inconsistent across SDKs — remove or confirm with Stripe/support and test.
- SJ, BV, TF, GS — ISO codes and not listed as blocked, but likely unnecessary for your use case; prefer a minimal list.
- Action: replace the large hardcoded list with only the markets you must support (e.g., {"IN"}) or a config-driven list; run a Checkout Session create test for any codes you retain or escalate to Stripe for XK.
Comment on lines
+102
to
+171
| func RechargeCreem(referenceId string, customerEmail string, customerName string) (err error) { | ||
| if referenceId == "" { | ||
| return errors.New("未提供支付单号") | ||
| } | ||
|
|
||
| var quota float64 | ||
| topUp := &TopUp{} | ||
|
|
||
| refCol := "`trade_no`" | ||
| if common.UsingPostgreSQL { | ||
| refCol = `"trade_no"` | ||
| } | ||
|
|
||
| err = DB.Transaction(func(tx *gorm.DB) error { | ||
| err := tx.Set("gorm:query_option", "FOR UPDATE").Where(refCol+" = ?", referenceId).First(topUp).Error | ||
| if err != nil { | ||
| return errors.New("充值订单不存在") | ||
| } | ||
|
|
||
| if topUp.Status != common.TopUpStatusPending { | ||
| return errors.New("充值订单状态错误") | ||
| } | ||
|
|
||
| topUp.CompleteTime = common.GetTimestamp() | ||
| topUp.Status = common.TopUpStatusSuccess | ||
| err = tx.Save(topUp).Error | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Creem 直接使用 Amount 作为充值额度 | ||
| quota = float64(topUp.Amount) | ||
|
|
||
| // 构建更新字段，优先使用邮箱，如果邮箱为空则使用用户名 | ||
| updateFields := map[string]interface{}{ | ||
| "quota": gorm.Expr("quota + ?", quota), | ||
| } | ||
|
|
||
| // 如果有客户邮箱，尝试更新用户邮箱（仅当用户邮箱为空时） | ||
| if customerEmail != "" { | ||
| // 先检查用户当前邮箱是否为空 | ||
| var user User | ||
| err = tx.Where("id = ?", topUp.UserId).First(&user).Error | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // 如果用户邮箱为空，则更新为支付时使用的邮箱 | ||
| if user.Email == "" { | ||
| updateFields["email"] = customerEmail | ||
| fmt.Printf("更新用户邮箱：用户ID %d, 新邮箱 %s\n", topUp.UserId, customerEmail) | ||
| } | ||
| } | ||
|
|
||
| err = tx.Model(&User{}).Where("id = ?", topUp.UserId).Updates(updateFields).Error | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| }) | ||
|
|
||
| if err != nil { | ||
| return errors.New("充值失败，" + err.Error()) | ||
| } | ||
|
|
||
| RecordLog(topUp.UserId, LogTypeTopup, fmt.Sprintf("使用Creem充值成功，充值额度: %v，支付金额：%.2f，客户邮箱：%s", common.FormatQuota(int(quota)), topUp.Money, customerEmail)) | ||
|
|
||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
Sanitize PII in logs; remove unused parameter; keep logging consistent.
- Logging customerEmail in plain text and using fmt.Printf in model layer is a privacy/compliance risk and inconsistent with existing logging patterns.
- customerName is unused.
Apply:
-func RechargeCreem(referenceId string, customerEmail string, customerName string) (err error) {
+func RechargeCreem(referenceId string, customerEmail string) (err error) {
@@
- if user.Email == "" {
- updateFields["email"] = customerEmail
- fmt.Printf("更新用户邮箱：用户ID %d, 新邮箱 %s\n", topUp.UserId, customerEmail)
- }
+ if user.Email == "" {
+ updateFields["email"] = customerEmail
+ }
@@
-RecordLog(topUp.UserId, LogTypeTopup, fmt.Sprintf("使用Creem充值成功，充值额度: %v，支付金额：%.2f，客户邮箱：%s", common.FormatQuota(int(quota)), topUp.Money, customerEmail))
+masked := customerEmail
+if at := strings.Index(customerEmail, "@"); at > 2 {
+ masked = customerEmail[:2] + "***" + customerEmail[at-1:]
+}
+RecordLog(topUp.UserId, LogTypeTopup, fmt.Sprintf("使用Creem充值成功，充值额度: %v，支付金额：%.2f，客户邮箱：%s", common.FormatQuota(int(quota)), topUp.Money, masked))If API compatibility requires the third arg, keep it but mark as unused: _ string.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func RechargeCreem(referenceId string, customerEmail string, customerName string) (err error) { | |
| if referenceId == "" { | |
| return errors.New("未提供支付单号") | |
| } | |
| var quota float64 | |
| topUp := &TopUp{} | |
| refCol := "`trade_no`" | |
| if common.UsingPostgreSQL { | |
| refCol = `"trade_no"` | |
| } | |
| err = DB.Transaction(func(tx *gorm.DB) error { | |
| err := tx.Set("gorm:query_option", "FOR UPDATE").Where(refCol+" = ?", referenceId).First(topUp).Error | |
| if err != nil { | |
| return errors.New("充值订单不存在") | |
| } | |
| if topUp.Status != common.TopUpStatusPending { | |
| return errors.New("充值订单状态错误") | |
| } | |
| topUp.CompleteTime = common.GetTimestamp() | |
| topUp.Status = common.TopUpStatusSuccess | |
| err = tx.Save(topUp).Error | |
| if err != nil { | |
| return err | |
| } | |
| // Creem 直接使用 Amount 作为充值额度 | |
| quota = float64(topUp.Amount) | |
| // 构建更新字段，优先使用邮箱，如果邮箱为空则使用用户名 | |
| updateFields := map[string]interface{}{ | |
| "quota": gorm.Expr("quota + ?", quota), | |
| } | |
| // 如果有客户邮箱，尝试更新用户邮箱（仅当用户邮箱为空时） | |
| if customerEmail != "" { | |
| // 先检查用户当前邮箱是否为空 | |
| var user User | |
| err = tx.Where("id = ?", topUp.UserId).First(&user).Error | |
| if err != nil { | |
| return err | |
| } | |
| // 如果用户邮箱为空，则更新为支付时使用的邮箱 | |
| if user.Email == "" { | |
| updateFields["email"] = customerEmail | |
| fmt.Printf("更新用户邮箱：用户ID %d, 新邮箱 %s\n", topUp.UserId, customerEmail) | |
| } | |
| } | |
| err = tx.Model(&User{}).Where("id = ?", topUp.UserId).Updates(updateFields).Error | |
| if err != nil { | |
| return err | |
| } | |
| return nil | |
| }) | |
| if err != nil { | |
| return errors.New("充值失败，" + err.Error()) | |
| } | |
| RecordLog(topUp.UserId, LogTypeTopup, fmt.Sprintf("使用Creem充值成功，充值额度: %v，支付金额：%.2f，客户邮箱：%s", common.FormatQuota(int(quota)), topUp.Money, customerEmail)) | |
| return nil | |
| } | |
| func RechargeCreem(referenceId string, customerEmail string) (err error) { | |
| if referenceId == "" { | |
| return errors.New("未提供支付单号") | |
| } | |
| var quota float64 | |
| topUp := &TopUp{} | |
| refCol := "`trade_no`" | |
| if common.UsingPostgreSQL { | |
| refCol = `"trade_no"` | |
| } | |
| err = DB.Transaction(func(tx *gorm.DB) error { | |
| err := tx.Set("gorm:query_option", "FOR UPDATE").Where(refCol+" = ?", referenceId).First(topUp).Error | |
| if err != nil { | |
| return errors.New("充值订单不存在") | |
| } | |
| if topUp.Status != common.TopUpStatusPending { | |
| return errors.New("充值订单状态错误") | |
| } | |
| topUp.CompleteTime = common.GetTimestamp() | |
| topUp.Status = common.TopUpStatusSuccess | |
| err = tx.Save(topUp).Error | |
| if err != nil { | |
| return err | |
| } | |
| // Creem 直接使用 Amount 作为充值额度 | |
| quota = float64(topUp.Amount) | |
| // 构建更新字段，优先使用邮箱，如果邮箱为空则使用用户名 | |
| updateFields := map[string]interface{}{ | |
| "quota": gorm.Expr("quota + ?", quota), | |
| } | |
| // 如果有客户邮箱，尝试更新用户邮箱（仅当用户邮箱为空时） | |
| if customerEmail != "" { | |
| // 先检查用户当前邮箱是否为空 | |
| var user User | |
| err = tx.Where("id = ?", topUp.UserId).First(&user).Error | |
| if err != nil { | |
| return err | |
| } | |
| // 如果用户邮箱为空，则更新为支付时使用的邮箱 | |
| if user.Email == "" { | |
| updateFields["email"] = customerEmail | |
| } | |
| } | |
| err = tx.Model(&User{}).Where("id = ?", topUp.UserId).Updates(updateFields).Error | |
| if err != nil { | |
| return err | |
| } | |
| return nil | |
| }) | |
| if err != nil { | |
| return errors.New("充值失败，" + err.Error()) | |
| } | |
| masked := customerEmail | |
| if at := strings.Index(customerEmail, "@"); at > 2 { | |
| masked = customerEmail[:2] + "***" + customerEmail[at-1:] | |
| } | |
| RecordLog(topUp.UserId, LogTypeTopup, fmt.Sprintf("使用Creem充值成功，充值额度: %v，支付金额：%.2f，客户邮箱：%s", common.FormatQuota(int(quota)), topUp.Money, masked)) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In model/topup.go around lines 102 to 171, the function logs customerEmail in
plain text and uses fmt.Printf in the model layer while customerName is unused;
remove the fmt.Printf call, stop writing raw PII into logs (either omit
customerEmail from logs or log a sanitized/masked version), and use the existing
logging pattern (RecordLog or the project logger) instead of fmt.Printf; if the
third parameter must remain for API compatibility, rename it to an unnamed
parameter (`_ string`) or mark it as unused to avoid compiler warnings.
| apiRouter.GET("/ratio_config", middleware.CriticalRateLimit(), controller.GetRatioConfig) | ||
|
|
||
| apiRouter.POST("/stripe/webhook", controller.StripeWebhook) | ||
| apiRouter.POST("/creem/webhook", controller.CreemWebhook) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Avoid rate-limiting webhook deliveries
This route sits under apiRouter which applies GlobalAPIRateLimit(). Webhooks can burst (e.g., retries), and rate-limiting here risks dropping valid events. Mount webhook routes on a group that does NOT use the global rate limit (Stripe is affected too).
Run to inspect the middleware and usage:
If needed, split webhook routes before applying GlobalAPIRateLimit():
func SetApiRouter(router *gin.Engine) {
- apiRouter := router.Group("/api")
- apiRouter.Use(gzip.Gzip(gzip.DefaultCompression))
- apiRouter.Use(middleware.GlobalAPIRateLimit())
+ // Webhooks: no global rate-limit
+ webhookRouter := router.Group("/api")
+ webhookRouter.Use(gzip.Gzip(gzip.DefaultCompression))
+ webhookRouter.POST("/stripe/webhook", controller.StripeWebhook)
+ webhookRouter.POST("/creem/webhook", controller.CreemWebhook)
+
+ apiRouter := router.Group("/api")
+ apiRouter.Use(gzip.Gzip(gzip.DefaultCompression))
+ apiRouter.Use(middleware.GlobalAPIRateLimit())🏁 Script executed:
#!/bin/bash
rg -n "GlobalAPIRateLimit\(" -C3
rg -n "CreemWebhook|StripeWebhook" -C2Length of output: 11983
Move webhook endpoints out of the global rate-limited /api group
router/api-router.go applies middleware.GlobalAPIRateLimit() to /api (line 14) while POST /stripe/webhook and POST /creem/webhook are registered under that group (lines 41–42); register those two webhook routes in a separate /api group that does NOT use GlobalAPIRateLimit() so webhooks aren’t dropped during bursts/retries.
func SetApiRouter(router *gin.Engine) {
- apiRouter := router.Group("/api")
- apiRouter.Use(gzip.Gzip(gzip.DefaultCompression))
- apiRouter.Use(middleware.GlobalAPIRateLimit())
+ // Webhooks: no global rate-limit
+ webhookRouter := router.Group("/api")
+ webhookRouter.Use(gzip.Gzip(gzip.DefaultCompression))
+ webhookRouter.POST("/stripe/webhook", controller.StripeWebhook)
+ webhookRouter.POST("/creem/webhook", controller.CreemWebhook)
+
+ apiRouter := router.Group("/api")
+ apiRouter.Use(gzip.Gzip(gzip.DefaultCompression))
+ apiRouter.Use(middleware.GlobalAPIRateLimit())router/api-router.go:12–16, 39–43.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| apiRouter.POST("/creem/webhook", controller.CreemWebhook) | |
| func SetApiRouter(router *gin.Engine) { | |
| // Webhooks: no global rate-limit | |
| webhookRouter := router.Group("/api") | |
| webhookRouter.Use(gzip.Gzip(gzip.DefaultCompression)) | |
| webhookRouter.POST("/stripe/webhook", controller.StripeWebhook) | |
| webhookRouter.POST("/creem/webhook", controller.CreemWebhook) | |
| apiRouter := router.Group("/api") | |
| apiRouter.Use(gzip.Gzip(gzip.DefaultCompression)) | |
| apiRouter.Use(middleware.GlobalAPIRateLimit()) | |
| // ... other apiRouter route registrations ... | |
| } |
🤖 Prompt for AI Agents
In router/api-router.go around lines 12–16 and 39–43, the POST /stripe/webhook
and POST /creem/webhook routes are registered inside the /api group that has
middleware.GlobalAPIRateLimit(), which can drop webhook requests; create a new
/api route group (or an /api/webhook subgroup) that does NOT apply
GlobalAPIRateLimit() and move the two webhook registrations (POST
/stripe/webhook and POST /creem/webhook) into that new rate-limit-free group so
webhooks are handled without global rate limiting.
Contributor
|
I don't think forcing the collection of user information is a good idea. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
controller/topup_stripe.go (1)
218-224: Unresolved merge conflict breaks build; pick one URL base and remove markers.Compilation will fail due to
<<<<<<</>>>>>>>markers and the undefinedsystem_settingreference.Apply:
-<<<<<<< Updated upstream - SuccessURL: stripe.String(setting.ServerAddress + "/log"), - CancelURL: stripe.String(setting.ServerAddress + "/topup"), -======= - SuccessURL: stripe.String(system_setting.ServerAddress + "/console/log"), - CancelURL: stripe.String(system_setting.ServerAddress + "/console/topup"), ->>>>>>> Stashed changes + SuccessURL: stripe.String(setting.ServerAddress + "/log"), + CancelURL: stripe.String(setting.ServerAddress + "/topup"),#!/bin/bash # Detect any leftover conflict markers and forbidden system_setting reference rg -n '^(<<<<<<<|=======|>>>>>>>)' rg -n '\bsystem_setting\.ServerAddress\b'
🧹 Nitpick comments (1)
controller/topup_stripe.go (1)
232-246: Gate full address/phone collection by market or config to address privacy concerns.Make collection conditional (e.g., only for countries requiring it) via config/env to avoid forcing extra PII globally.
I can draft a config-driven approach (e.g.,
StripeCollectFullAddressCountries=["IN"]) and wire it here—let me know.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
controller/topup_stripe.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
controller/topup_stripe.go (2)
setting/system_setting.go (1)
ServerAddress(3-3)setting/payment_stripe.go (1)
StripePriceId(5-5)
Comment on lines
+232
to
246
| ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{ | ||
| BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)), | ||
| CustomFields: []*stripe.CheckoutSessionCustomFieldParams{ | ||
| { | ||
| Key: stripe.String("customer_phone"), | ||
| Label: &stripe.CheckoutSessionCustomFieldLabelParams{ | ||
| Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)), | ||
| Custom: stripe.String("Phone Number"), | ||
| }, | ||
| Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)), | ||
| Optional: stripe.Bool(false), | ||
| }, | ||
| }, | ||
| Metadata: map[string]string{}, | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Mis-nested fields and missing AllowedCountries: compile/runtime break.
BillingAddressCollection,CustomFields, andMetadataare incorrectly placed insideShippingAddressCollection(won’t compile against stripe-go v81).ShippingAddressCollection.AllowedCountriesis required; omitting it causes Checkout Session creation to fail.- A closing brace for
ShippingAddressCollectionis missing.
Fix structure and set minimal countries (e.g., India only) and enable the standard phone field.
Apply:
- ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{
- BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)),
- CustomFields: []*stripe.CheckoutSessionCustomFieldParams{
- {
- Key: stripe.String("customer_phone"),
- Label: &stripe.CheckoutSessionCustomFieldLabelParams{
- Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)),
- Custom: stripe.String("Phone Number"),
- },
- Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)),
- Optional: stripe.Bool(false),
- },
- },
- Metadata: map[string]string{},
- }
+ BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)),
+ ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{
+ AllowedCountries: stripe.StringSlice([]string{"IN"}),
+ },
+ PhoneNumberCollection: &stripe.CheckoutSessionPhoneNumberCollectionParams{
+ Enabled: stripe.Bool(true),
+ },
+ Metadata: map[string]string{},Optional (if you still want a free‑text field in addition to the validated phone): add this at the same level (not inside ShippingAddressCollection):
+ CustomFields: []*stripe.CheckoutSessionCustomFieldParams{
+ {
+ Key: stripe.String("customer_phone_extra"),
+ Label: &stripe.CheckoutSessionCustomFieldLabelParams{
+ Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)),
+ Custom: stripe.String("Alternate Phone"),
+ },
+ Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)),
+ Optional: stripe.Bool(true),
+ },
+ },🏁 Script executed:
#!/bin/bash
# Ensure required fields exist at correct levels
rg -n 'BillingAddressCollection:\s*stripe\.String' controller/topup_stripe.go -n
rg -n 'ShippingAddressCollection:\s*&stripe\.CheckoutSessionShippingAddressCollectionParams' -n controller/topup_stripe.go -A3
rg -n 'AllowedCountries:\s*stripe\.StringSlice' controller/topup_stripe.go -nLength of output: 383
Fix mis-nested Stripe CheckoutSession params in controller/topup_stripe.go (lines 232–246).
- BillingAddressCollection, CustomFields and Metadata are currently nested inside ShippingAddressCollection (rg shows BillingAddressCollection at line 233 while ShippingAddressCollection starts at 232) — move them to the top-level of the CheckoutSession params.
- ShippingAddressCollection must include AllowedCountries and the struct literal needs a closing brace; also enable PhoneNumberCollection for the validated phone field.
Apply:
- ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{
- BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)),
- CustomFields: []*stripe.CheckoutSessionCustomFieldParams{
- {
- Key: stripe.String("customer_phone"),
- Label: &stripe.CheckoutSessionCustomFieldLabelParams{
- Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)),
- Custom: stripe.String("Phone Number"),
- },
- Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)),
- Optional: stripe.Bool(false),
- },
- },
- Metadata: map[string]string{},
- }
+ BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)),
+ ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{
+ AllowedCountries: stripe.StringSlice([]string{"IN"}),
+ },
+ PhoneNumberCollection: &stripe.CheckoutSessionPhoneNumberCollectionParams{
+ Enabled: stripe.Bool(true),
+ },
+ Metadata: map[string]string{},Optional (if you still want a free-text field in addition to the validated phone): add at the same level (not inside ShippingAddressCollection):
+ CustomFields: []*stripe.CheckoutSessionCustomFieldParams{
+ {
+ Key: stripe.String("customer_phone_extra"),
+ Label: &stripe.CheckoutSessionCustomFieldLabelParams{
+ Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)),
+ Custom: stripe.String("Alternate Phone"),
+ },
+ Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)),
+ Optional: stripe.Bool(true),
+ },
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{ | |
| BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)), | |
| CustomFields: []*stripe.CheckoutSessionCustomFieldParams{ | |
| { | |
| Key: stripe.String("customer_phone"), | |
| Label: &stripe.CheckoutSessionCustomFieldLabelParams{ | |
| Type: stripe.String(string(stripe.CheckoutSessionCustomFieldLabelTypeCustom)), | |
| Custom: stripe.String("Phone Number"), | |
| }, | |
| Type: stripe.String(string(stripe.CheckoutSessionCustomFieldTypeText)), | |
| Optional: stripe.Bool(false), | |
| }, | |
| }, | |
| Metadata: map[string]string{}, | |
| } | |
| BillingAddressCollection: stripe.String(string(stripe.CheckoutSessionBillingAddressCollectionRequired)), | |
| ShippingAddressCollection: &stripe.CheckoutSessionShippingAddressCollectionParams{ | |
| AllowedCountries: stripe.StringSlice([]string{"IN"}), | |
| }, | |
| PhoneNumberCollection: &stripe.CheckoutSessionPhoneNumberCollectionParams{ | |
| Enabled: stripe.Bool(true), | |
| }, | |
| Metadata: map[string]string{}, |
🤖 Prompt for AI Agents
In controller/topup_stripe.go around lines 232–246 the CheckoutSession params
are mis-nested: BillingAddressCollection, CustomFields and Metadata are placed
inside ShippingAddressCollection; move BillingAddressCollection, CustomFields
and Metadata up to the top-level of the CheckoutSession params object (same
indentation as ShippingAddressCollection). Ensure ShippingAddressCollection is
its own struct literal with a closing brace and include AllowedCountries (e.g.,
[]string{"US", "CA"} or project-specific countries) and enable
PhoneNumberCollection (set to true) so the validated phone field is collected;
if you still want an extra free-text phone field, add it as a separate
CustomField at the same top level (not inside ShippingAddressCollection).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some country like India require customers' full details during. With this PR its possible during checkout with stripe.
Summary by CodeRabbit
New Features
Improvements