feat(billing): audit customer portal session creation#1714
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new audit event constant 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Coverage Report for CI Build 28435609189Coverage increased (+0.03%) to 43.819%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/api/v1beta1connect/billing_checkout_test.go (1)
378-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the non-fatal audit-failure path.
The PR's key behavioral guarantee is that a failing audit emission is logged but does not fail the portal request. None of the tests exercise
auditRecordService.Createreturning an error. Consider a case whereCreatereturns an error and assertCreateCheckoutstill returns the session withNoError.💚 Suggested test
func TestConnectHandler_CreateCheckout_CustomerPortalAuditFailureNonFatal(t *testing.T) { mockCheckoutSvc := mocks.NewCheckoutService(t) mockCustomerSvc := mocks.NewCustomerService(t) mockAuditSvc := mocks.NewAuditRecordService(t) mockCustomerSvc.EXPECT().GetByOrgID(mock.Anything, "org-123"). Return(customer.Customer{ID: "customer-123", OrgID: "org-123"}, nil) mockCheckoutSvc.EXPECT().CreateSessionForCustomerPortal(mock.Anything, mock.Anything). Return(testCheckout, nil) mockAuditSvc.EXPECT().Create(mock.Anything, mock.Anything). Return(auditrecord.AuditRecord{}, false, errors.New("audit error")) h := &ConnectHandler{ checkoutService: mockCheckoutSvc, customerService: mockCustomerSvc, auditRecordService: mockAuditSvc, } got, err := h.CreateCheckout(context.Background(), connect.NewRequest(&frontierv1beta1.CreateCheckoutRequest{ OrgId: "org-123", SuccessUrl: "https://example.com/success", CancelUrl: "https://example.com/cancel", SetupBody: &frontierv1beta1.CheckoutSetupBody{CustomerPortal: true}, })) assert.NoError(t, err) assert.Equal(t, testCheckoutID, got.Msg.GetCheckoutSession().GetId()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/v1beta1connect/billing_checkout_test.go` around lines 378 - 414, Add a test for the non-fatal audit failure path in CreateCheckout: in the CustomerPortal flow, make auditRecordService.Create return an error and verify ConnectHandler.CreateCheckout still succeeds with NoError and returns the checkout session. Reuse the existing TestConnectHandler_CreateCheckout_CustomerPortalAuditRecord setup and the same symbols (ConnectHandler, CreateCheckout, auditRecordService.Create) to locate the behavior, but change the assertion to confirm the audit error is swallowed rather than bubbling up.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/api/v1beta1connect/billing_checkout_test.go`:
- Around line 378-414: Add a test for the non-fatal audit failure path in
CreateCheckout: in the CustomerPortal flow, make auditRecordService.Create
return an error and verify ConnectHandler.CreateCheckout still succeeds with
NoError and returns the checkout session. Reuse the existing
TestConnectHandler_CreateCheckout_CustomerPortalAuditRecord setup and the same
symbols (ConnectHandler, CreateCheckout, auditRecordService.Create) to locate
the behavior, but change the assertion to confirm the audit error is swallowed
rather than bubbling up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a4f0fdc-290a-4a56-94ce-d451a16bc4ce
📒 Files selected for processing (3)
internal/api/v1beta1connect/billing_checkout.gointernal/api/v1beta1connect/billing_checkout_test.gopkg/auditrecord/consts.go
rohilsurana
force-pushed
the
feat/audit-billing-portal-session
branch
from
4406e27 to
ed847be
Compare
rohilsurana
force-pushed
the
feat/audit-billing-portal-session
branch
from
ed847be to
760a54c
Compare
3 participants
What
Emit an audit record whenever a Stripe customer portal session is created via
FrontierService/CreateCheckout(setupBody.customerPortal = true).The portal is how billing details (address, tax IDs, etc.) get updated. Recording every session creation lets us trace who opened the portal to update — or attempt to update — billing details, including whether the actor was a platform super-user.
How
billing_customer.portal_session_createdinpkg/auditrecord/consts.go.CreateCheckouthandler's customer-portal branch, emit an audit record after the session is created successfully:checkout_idin metadata.is_super_userflag are auto-enriched from request context (same mechanism as role/PAT audit records).This covers both the client app flow and the upcoming admin-app super-user flow, since both go through the same RPC.
Notes
CreateCheckoutauthz via theplatform->superuserschema cascade, and the service has no KYC gate.Test plan
TestConnectHandler_CreateCheckout_CustomerPortalAuditRecordasserts the record is emitted with the correct event, org, and billing-customer target.CreateCheckouttable tests updated to allow the emission.go test ./internal/api/v1beta1connect/...andgolangci-lintpass.