From f8575b8b0d6489646c02e289fb77eaaf5c3b7d5f Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 10 Aug 2026 10:23:57 +0530 Subject: [PATCH] feat(deleter): clear or report org delete blockers up front The delete first checks everything that blocks it and returns all the reasons together as one failed_precondition response: a running subscription on a paid plan (the caller downgrades it to the standard plan), unpaid (open or uncollectible) invoices, and a negative token balance which support has to settle. When nothing blocks, subscriptions still running on the standard plan are canceled immediately with unbilled usage invoiced on the spot, and the invoice check runs again so a final invoice still blocks the delete. Unused tokens do not block: the delete forfeits them and writes the amount to an audit record. An already-deleted org returns not found before any checks run. Invoice checks always sync from the billing provider first so the decision is made on fresh data. --- billing/invoice/invoice.go | 3 + cmd/serve.go | 2 +- core/audit/audit.go | 2 + core/deleter/deleter.go | 37 ++- core/deleter/service.go | 212 +++++++++++++- core/deleter/service_test.go | 300 +++++++++++++++++++- go.mod | 2 +- internal/api/v1beta1connect/deleter.go | 34 +++ internal/api/v1beta1connect/deleter_test.go | 49 ++++ 9 files changed, 610 insertions(+), 31 deletions(-) diff --git a/billing/invoice/invoice.go b/billing/invoice/invoice.go index d40f2f3534..f8814f9d8f 100644 --- a/billing/invoice/invoice.go +++ b/billing/invoice/invoice.go @@ -43,6 +43,9 @@ const ( DraftState State = "draft" OpenState State = "open" PaidState State = "paid" + // UncollectibleState marks an invoice the provider has written off; it + // can still be paid. + UncollectibleState State = "uncollectible" ) type Invoice struct { diff --git a/cmd/serve.go b/cmd/serve.go index 346d9ecad2..0146b8c502 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -587,7 +587,7 @@ func buildAPIDependencies( cascadeDeleter := deleter.NewCascadeDeleter(organizationService, projectService, resourceService, groupService, membershipService, policyService, roleService, invitationService, userService, userPATService, serviceUserService, customerService, subscriptionService, invoiceService, checkoutService, - creditService, orgKycService, + creditService, orgKycService, planService, cfg.Billing.AccountConfig.DefaultPlan, ) // we should default it with a stdout logger repository as postgres can start to bloat really fast diff --git a/core/audit/audit.go b/core/audit/audit.go index d16d501863..c68b896b17 100644 --- a/core/audit/audit.go +++ b/core/audit/audit.go @@ -97,6 +97,7 @@ const ( BillingAccountDetailsUpdatedEvent EventName = "app.billing.account.details.updated" BillingCheckoutDeletedEvent EventName = "app.billing.checkout.deleted" + BillingTokensForfeitedEvent EventName = "app.billing.tokens.forfeited" ) var systemEvents = []EventName{ @@ -113,6 +114,7 @@ var systemEvents = []EventName{ OrgDeletedEvent, OrgDisabledEvent, BillingCheckoutDeletedEvent, + BillingTokensForfeitedEvent, } func IsSystemEvent(event EventName) bool { diff --git a/core/deleter/deleter.go b/core/deleter/deleter.go index cad34469c3..63eea45d4d 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -1,7 +1,38 @@ package deleter -import "fmt" +import "strings" -var ( - ErrDeleteNotAllowed = fmt.Errorf("deletion not allowed for billed accounts") +// Reasons an organization delete can be blocked. The API error carries +// them as violation types, so a client can tell the reasons apart +// without reading the message text. +const ( + BlockerActiveSubscription = "ACTIVE_SUBSCRIPTION" + BlockerUnpaidInvoice = "UNPAID_INVOICE" + BlockerNegativeTokenBalance = "NEGATIVE_TOKEN_BALANCE" ) + +// Blocker is one reason an organization cannot be deleted right now. +type Blocker struct { + // Type is one of the Blocker* constants. + Type string + // Subject is the id of the entity behind the reason. + Subject string + // Message says what blocks the delete and how to clear it. + Message string +} + +// BlockedError carries every blocker the up-front check found, so the +// caller gets one checklist instead of discovering blockers one retry at +// a time. +type BlockedError struct { + OrgID string + Blockers []Blocker +} + +func (e *BlockedError) Error() string { + msgs := make([]string, 0, len(e.Blockers)) + for _, b := range e.Blockers { + msgs = append(msgs, b.Message) + } + return "organization cannot be deleted yet: " + strings.Join(msgs, "; ") +} diff --git a/core/deleter/service.go b/core/deleter/service.go index fb4be96ebc..b463b7a58d 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "log/slog" + "slices" + "strconv" "github.com/raystack/frontier/core/audit" @@ -16,6 +18,10 @@ import ( "github.com/raystack/frontier/billing/customer" + "github.com/raystack/frontier/billing/plan" + + "github.com/raystack/frontier/billing/subscription" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/internal/bootstrap/schema" @@ -34,10 +40,6 @@ import ( "github.com/raystack/frontier/core/serviceuser" ) -const ( - DisableDeleteIfBilled = true -) - type ProjectService interface { List(ctx context.Context, flt project.Filter) ([]project.Project, error) DeleteModel(ctx context.Context, id string) error @@ -98,11 +100,14 @@ type CustomerService interface { } type SubscriptionService interface { + List(ctx context.Context, filter subscription.Filter) ([]subscription.Subscription, error) + Cancel(ctx context.Context, id string, immediate bool) (subscription.Subscription, error) DeleteByCustomer(ctx context.Context, customr customer.Customer) error } type InvoiceService interface { List(ctx context.Context, flt invoice.Filter) ([]invoice.Invoice, error) + SyncWithProvider(ctx context.Context, customr customer.Customer) error DeleteByCustomer(ctx context.Context, customr customer.Customer) error } @@ -112,6 +117,7 @@ type CheckoutService interface { } type CreditService interface { + GetBalance(ctx context.Context, accountID string) (int64, error) DeleteByAccountID(ctx context.Context, accountID string) error } @@ -119,6 +125,10 @@ type KycService interface { DeleteKyc(ctx context.Context, orgID string) error } +type PlanService interface { + GetByID(ctx context.Context, id string) (plan.Plan, error) +} + type Service struct { projService ProjectService orgService OrganizationService @@ -137,6 +147,13 @@ type Service struct { checkoutService CheckoutService creditService CreditService kycService KycService + planService PlanService + // defaultPlan is the billing.customer.default_plan config value: the + // plan orgs start on. It holds the plan's name or its uuid, the lookup + // resolves both. A subscription on this plan may still be running when + // the org is deleted; any other plan blocks the delete until the + // caller downgrades it + defaultPlan string } func NewCascadeDeleter(orgService OrganizationService, projService ProjectService, @@ -148,7 +165,8 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic serviceUserService ServiceUserService, customerService CustomerService, subService SubscriptionService, invoiceService InvoiceService, checkoutService CheckoutService, - creditService CreditService, kycService KycService) *Service { + creditService CreditService, kycService KycService, + planService PlanService, defaultPlan string) *Service { return &Service{ projService: projService, orgService: orgService, @@ -167,6 +185,8 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic checkoutService: checkoutService, creditService: creditService, kycService: kycService, + planService: planService, + defaultPlan: defaultPlan, } } @@ -217,9 +237,16 @@ func (d Service) DeleteGroup(ctx context.Context, id string) error { // leaves the org owned and the delete can simply be run again. Every step // treats already-deleted data as success for the same reason. func (d Service) DeleteOrganization(ctx context.Context, id string) error { - // check if delete is allowed - if err := d.canDelete(ctx, id); err != nil { - return fmt.Errorf("%s: %w", err.Error(), ErrDeleteNotAllowed) + // an org that is already gone has nothing left to check or tear down; + // disabled orgs stay deletable + if _, err := d.orgService.Get(ctx, id); err != nil && !errors.Is(err, organization.ErrDisabled) { + return err + } + + // clear what we can and collect what still blocks the delete, before + // touching any data + if err := d.ensureDeletable(ctx, id); err != nil { + return err } // delete all billing accounts @@ -364,6 +391,22 @@ func (d Service) DeleteCustomers(ctx context.Context, id string) error { slog.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingCheckoutDeletedEvent, "checkout_id", ch.ID) } } + // tokens still on the account are forfeited by this delete, so + // record the amount before the transactions are removed + balance, err := d.creditService.GetBalance(ctx, c.ID) + if err != nil { + return fmt.Errorf("failed to delete org while checking balance of billing account[%s]: %w", c.ID, err) + } + if balance > 0 { + if err := auditLogger.LogWithAttrs(audit.BillingTokensForfeitedEvent, audit.Target{ + ID: c.ID, + Type: "billing_account", + }, map[string]string{ + "amount": strconv.FormatInt(balance, 10), + }); err != nil { + slog.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.BillingTokensForfeitedEvent, "customer_id", c.ID) + } + } if err := d.creditService.DeleteByAccountID(ctx, c.ID); err != nil { return fmt.Errorf("failed to delete org while deleting a billing account transactions[%s]: %w", c.ID, err) } @@ -412,8 +455,26 @@ func (d Service) DeleteUser(ctx context.Context, userID string) error { return d.userService.Delete(ctx, userID) } -func (d Service) canDelete(ctx context.Context, id string) error { - // check if any invoice is present for customer +// ensureDeletable collects everything that blocks deleting the organization and +// returns it all as one BlockedError, so the caller gets a full checklist +// instead of discovering blockers one retry at a time. +// +// A running subscription on a paid plan blocks the delete: the caller has +// to downgrade it to the standard (default) plan first. A subscription on +// the standard plan is not a blocker — once no blockers are left, +// ensureDeletable cancels it itself, before any deletion starts. The cancel +// is immediate and bills unbilled usage on the spot, so the invoice check +// runs once more after it: a final invoice created by the cancel still +// blocks the delete until it is paid. +// +// Unused tokens do not block either: the delete forfeits them. The client +// gets the caller's confirmation before sending the delete, and the +// forfeited amount is written to an audit record during teardown. +// +// Accounts without a billing provider are only checked for token balances: +// their subscription and invoice rows have nothing behind them the caller +// could cancel or pay. +func (d Service) ensureDeletable(ctx context.Context, id string) error { customers, err := d.customerService.List(ctx, customer.Filter{ OrgID: id, }) @@ -421,14 +482,135 @@ func (d Service) canDelete(ctx context.Context, id string) error { return err } + // resolve the one plan a running subscription may still be on; any other + // active plan must be downgraded by the caller first + standardPlanID, err := d.standardPlanID(ctx, customers) + if err != nil { + return err + } + + var blockers []Blocker + for _, c := range customers { + if !c.IsOffline() { + subs, err := d.subService.List(ctx, subscription.Filter{CustomerID: c.ID}) + if err != nil { + return fmt.Errorf("failed to check subscriptions for billing account[%s]: %w", c.ID, err) + } + for _, sub := range subs { + if sub.IsActive() && sub.PlanID != standardPlanID { + blockers = append(blockers, Blocker{ + Type: BlockerActiveSubscription, + Subject: sub.ID, + Message: fmt.Sprintf("subscription[%s] is %s on a paid plan: downgrade to the standard plan, then retry the delete", sub.ID, sub.State), + }) + } + } + + bs, err := d.invoiceBlockers(ctx, c) + if err != nil { + return err + } + blockers = append(blockers, bs...) + } + + balance, err := d.creditService.GetBalance(ctx, c.ID) + if err != nil { + return fmt.Errorf("failed to check token balance of billing account[%s]: %w", c.ID, err) + } + // the balance goes below zero when the account has an overdraft + // floor (credit_min under zero, the postpaid setup) and tokens were + // spent on credit. That debt is money owed, so it must be settled + // before the org can go + if balance < 0 { + blockers = append(blockers, Blocker{ + Type: BlockerNegativeTokenBalance, + Subject: c.ID, + Message: fmt.Sprintf("billing account[%s] owes %d tokens: contact support to settle the balance, then retry the delete", c.ID, -balance), + }) + } + } + if len(blockers) > 0 { + return &BlockedError{OrgID: id, Blockers: blockers} + } + + // no blockers were found, so the delete will happen. The only + // subscriptions still running are on the standard plan: cancel them, + // then check the invoices again because the cancel may have created a + // final one. This runs only after every blocker is clear, so a blocked + // delete never costs the caller their subscription for _, c := range customers { - if invoices, err := d.invoiceService.List(ctx, invoice.Filter{CustomerID: c.ID}); err != nil { - return fmt.Errorf("failed to check invoices for billing account[%s]: %w", c.ID, err) - } else if len(invoices) > 0 { - if DisableDeleteIfBilled { - return fmt.Errorf("cannot delete organization with billing account[%s]", c.ID) + if c.IsOffline() { + continue + } + subs, err := d.subService.List(ctx, subscription.Filter{CustomerID: c.ID}) + if err != nil { + return fmt.Errorf("failed to check subscriptions for billing account[%s]: %w", c.ID, err) + } + canceled := false + for _, sub := range subs { + if !sub.IsActive() { + continue + } + if _, err := d.subService.Cancel(ctx, sub.ID, true); err != nil { + return fmt.Errorf("failed to cancel subscription[%s] of billing account[%s]: %w", sub.ID, c.ID, err) } + canceled = true } + if canceled { + bs, err := d.invoiceBlockers(ctx, c) + if err != nil { + return err + } + blockers = append(blockers, bs...) + } + } + if len(blockers) > 0 { + return &BlockedError{OrgID: id, Blockers: blockers} } return nil } + +// standardPlanID resolves the configured default plan to its id. Without a +// configured default plan every active subscription blocks the delete. The +// lookup is skipped when no billing account talks to a provider. +func (d Service) standardPlanID(ctx context.Context, customers []customer.Customer) (string, error) { + if d.defaultPlan == "" { + return "", nil + } + if !slices.ContainsFunc(customers, func(c customer.Customer) bool { return !c.IsOffline() }) { + return "", nil + } + standardPlan, err := d.planService.GetByID(ctx, d.defaultPlan) + if err != nil { + return "", fmt.Errorf("failed to resolve the default plan[%s]: %w", d.defaultPlan, err) + } + return standardPlan.ID, nil +} + +// invoiceBlockers returns a blocker for every invoice of the account the +// caller can still pay; paid, void, and draft invoices don't block. The +// billing provider keeps its own permanent copy of every invoice, so +// deleting our rows later loses nothing. +func (d Service) invoiceBlockers(ctx context.Context, c customer.Customer) ([]Blocker, error) { + // the local invoice rows sync from the provider on a timer, so pull + // them fresh first: a just-paid invoice must not block, a just-created + // one must + if err := d.invoiceService.SyncWithProvider(ctx, c); err != nil { + return nil, fmt.Errorf("failed to sync invoices for billing account[%s]: %w", c.ID, err) + } + invoices, err := d.invoiceService.List(ctx, invoice.Filter{CustomerID: c.ID, NonZeroOnly: true}) + if err != nil { + return nil, fmt.Errorf("failed to check invoices for billing account[%s]: %w", c.ID, err) + } + var blockers []Blocker + for _, inv := range invoices { + if inv.State == invoice.OpenState || inv.State == invoice.UncollectibleState { + blockers = append(blockers, Blocker{ + Type: BlockerUnpaidInvoice, + Subject: inv.ID, + Message: fmt.Sprintf("invoice[%s] is unpaid: pay it via its hosted payment page, then retry the delete", inv.ID), + }) + } + } + return blockers, nil +} diff --git a/core/deleter/service_test.go b/core/deleter/service_test.go index 2e96132838..3b94082647 100644 --- a/core/deleter/service_test.go +++ b/core/deleter/service_test.go @@ -9,10 +9,13 @@ import ( "github.com/raystack/frontier/billing/checkout" "github.com/raystack/frontier/billing/customer" "github.com/raystack/frontier/billing/invoice" + "github.com/raystack/frontier/billing/plan" + "github.com/raystack/frontier/billing/subscription" "github.com/raystack/frontier/core/deleter" "github.com/raystack/frontier/core/deleter/mocks" "github.com/raystack/frontier/core/group" "github.com/raystack/frontier/core/invitation" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/core/project" "github.com/raystack/frontier/core/resource" @@ -41,11 +44,12 @@ type deleterMocks struct { checkoutSvc *mocks.CheckoutService creditSvc *mocks.CreditService kycSvc *mocks.KycService + planSvc *mocks.PlanService } func newMocks(t *testing.T) deleterMocks { t.Helper() - return deleterMocks{ + m := deleterMocks{ orgSvc: mocks.NewOrganizationService(t), projSvc: mocks.NewProjectService(t), resSvc: mocks.NewResourceService(t), @@ -63,13 +67,20 @@ func newMocks(t *testing.T) deleterMocks { checkoutSvc: mocks.NewCheckoutService(t), creditSvc: mocks.NewCreditService(t), kycSvc: mocks.NewKycService(t), + planSvc: mocks.NewPlanService(t), } + // the standard plan resolves on any org with provider-backed billing; + // stub the lookup once for every test + m.planSvc.EXPECT().GetByID(mock.Anything, "standard"). + Return(plan.Plan{ID: "plan-std"}, nil).Maybe() + return m } func (m deleterMocks) build() *deleter.Service { return deleter.NewCascadeDeleter(m.orgSvc, m.projSvc, m.resSvc, m.grpSvc, m.mbrSvc, m.polSvc, m.roleSvc, m.invSvc, m.usrSvc, m.patSvc, m.suSvc, - m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc) + m.custSvc, m.subSvc, m.invocSvc, m.checkoutSvc, m.creditSvc, m.kycSvc, + m.planSvc, "standard") } func TestDeleteProject(t *testing.T) { @@ -130,12 +141,19 @@ func TestDeleteOrganization(t *testing.T) { t.Run("full cascade delete", func(t *testing.T) { m := newMocks(t) - // canDelete and DeleteCustomers both list customers + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + + // the up-front check and DeleteCustomers both list customers c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{c}, nil) - m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). - Return([]invoice.Invoice{}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{{ID: "inv-1", State: invoice.PaidState}}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "canceled"}}, nil) // billing teardown m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) @@ -192,21 +210,250 @@ func TestDeleteOrganization(t *testing.T) { assert.NoError(t, err) }) - t.Run("blocked when billed customer has invoices", func(t *testing.T) { + t.Run("already deleted org returns not found without touching anything", func(t *testing.T) { + m := newMocks(t) + + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{}, organization.ErrNotExist) + // strict mocks: no other service may be called + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.ErrorIs(t, err, organization.ErrNotExist) + }) + + t.Run("collects all blockers in one error", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{ + {ID: "sub-1", State: "active", PlanID: "plan-paid"}, + {ID: "sub-2", State: "canceled", PlanID: "plan-paid"}, + }, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{ + {ID: "inv-1", State: invoice.OpenState}, + {ID: "inv-2", State: invoice.PaidState}, + }, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(-50, nil) + // strict mocks: nothing may be deleted and no subscription may be + // canceled on a blocked delete + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Equal(t, "org-1", blocked.OrgID) + types := make([]string, 0, len(blocked.Blockers)) + for _, b := range blocked.Blockers { + types = append(types, b.Type) + } + assert.Equal(t, []string{ + deleter.BlockerActiveSubscription, + deleter.BlockerUnpaidInvoice, + deleter.BlockerNegativeTokenBalance, + }, types) + }) + + t.Run("paid plan subscription blocks until the caller downgrades", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-paid"}}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + // strict mocks: the paid subscription must not be canceled + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerActiveSubscription, blocked.Blockers[0].Type) + assert.Contains(t, blocked.Blockers[0].Message, "downgrade to the standard plan") + }) + + t.Run("negative token balance blocks the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(-50, nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type) + assert.Contains(t, blocked.Blockers[0].Message, "contact support") + }) + + t.Run("unused tokens do not block the delete", func(t *testing.T) { m := newMocks(t) + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). - Return([]customer.Customer{{ID: "cust-1", ProviderID: "stripe-1"}}, nil) - m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). - Return([]invoice.Invoice{{ID: "inv-1"}}, nil) + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) + + m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). + Return([]checkout.Checkout{}, nil) + m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) + m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) err := m.build().DeleteOrganization(context.Background(), "org-1") - assert.ErrorIs(t, err, deleter.ErrDeleteNotAllowed) + assert.NoError(t, err) + }) + + t.Run("running subscription is canceled by the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{ + {ID: "sub-1", State: "active", PlanID: "plan-std"}, + {ID: "sub-2", State: "canceled", PlanID: "plan-paid"}, + }, nil) + // only the running standard-plan subscription is canceled, immediately + m.subSvc.EXPECT().Cancel(mock.Anything, "sub-1", true). + Return(subscription.Subscription{ID: "sub-1", State: "canceled"}, nil) + + m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). + Return([]checkout.Checkout{}, nil) + m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) + m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) + m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) + + m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). + Return([]project.Project{}, nil) + m.grpSvc.EXPECT().List(mock.Anything, group.Filter{OrganizationID: "org-1"}). + Return([]group.Group{}, nil) + m.suSvc.EXPECT().List(mock.Anything, serviceuser.Filter{OrgID: "org-1"}). + Return([]serviceuser.ServiceUser{}, nil) + m.invSvc.EXPECT().List(mock.Anything, invitation.Filter{OrgID: "org-1"}). + Return([]invitation.Invitation{}, nil) + m.kycSvc.EXPECT().DeleteKyc(mock.Anything, "org-1").Return(nil) + m.polSvc.EXPECT().List(mock.Anything, policy.Filter{OrgID: "org-1"}). + Return([]policy.Policy{}, nil) + m.roleSvc.EXPECT().List(mock.Anything, role.Filter{OrgID: "org-1"}). + Return([]role.Role{}, nil) + m.orgSvc.EXPECT().DeleteModel(mock.Anything, "org-1").Return(nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + assert.NoError(t, err) + }) + + t.Run("final invoice from the cancellation blocks the delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + // no unpaid invoice before the cancel, one after it + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{}, nil).Once() + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{{ID: "sub-1", State: "active", PlanID: "plan-std"}}, nil) + m.subSvc.EXPECT().Cancel(mock.Anything, "sub-1", true). + Return(subscription.Subscription{ID: "sub-1", State: "canceled"}, nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). + Return([]invoice.Invoice{{ID: "inv-final", State: invoice.OpenState}}, nil).Once() + // strict mocks: nothing may be deleted + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerUnpaidInvoice, blocked.Blockers[0].Type) + assert.Equal(t, "inv-final", blocked.Blockers[0].Subject) + }) + + t.Run("offline account only gets token checks", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-offline", ProviderID: ""} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + // strict mocks: no subscription or invoice call may happen + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-offline").Return(-10, nil) + + err := m.build().DeleteOrganization(context.Background(), "org-1") + + var blocked *deleter.BlockedError + assert.ErrorAs(t, err, &blocked) + assert.Len(t, blocked.Blockers, 1) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, blocked.Blockers[0].Type) }) t.Run("kyc delete failure keeps owner policies", func(t *testing.T) { m := newMocks(t) + // a disabled org is still deletable + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{}, organization.ErrDisabled) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -229,10 +476,16 @@ func TestDeleteOrganization(t *testing.T) { m := newMocks(t) c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{c}, nil) - m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1"}). + m.invocSvc.EXPECT().SyncWithProvider(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().List(mock.Anything, invoice.Filter{CustomerID: "cust-1", NonZeroOnly: true}). Return([]invoice.Invoice{}, nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(0, nil) + m.subSvc.EXPECT().List(mock.Anything, subscription.Filter{CustomerID: "cust-1"}). + Return([]subscription.Subscription{}, nil) m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c). Return(errors.New("provider is down")) // strict mocks: no policy, project, group, or org deletion may happen @@ -244,6 +497,8 @@ func TestDeleteOrganization(t *testing.T) { t.Run("propagates error when service user list fails", func(t *testing.T) { m := newMocks(t) + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -260,6 +515,8 @@ func TestDeleteOrganization(t *testing.T) { t.Run("propagates error when service user delete fails", func(t *testing.T) { m := newMocks(t) + m.orgSvc.EXPECT().Get(mock.Anything, "org-1"). + Return(organization.Organization{ID: "org-1"}, nil) m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). Return([]customer.Customer{}, nil) m.projSvc.EXPECT().List(mock.Anything, project.Filter{OrgID: "org-1"}). @@ -291,6 +548,7 @@ func TestDeleteCustomers(t *testing.T) { {ID: "chk-2", ProviderID: "cs_2", CustomerID: "cust-1", State: "expired"}, }, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1").Return(100, nil) m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-1").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-1").Return(nil) @@ -298,6 +556,25 @@ func TestDeleteCustomers(t *testing.T) { assert.NoError(t, err) }) + t.Run("balance check failure stops the customer delete", func(t *testing.T) { + m := newMocks(t) + + c := customer.Customer{ID: "cust-1", ProviderID: "stripe-1"} + m.custSvc.EXPECT().List(mock.Anything, customer.Filter{OrgID: "org-1"}). + Return([]customer.Customer{c}, nil) + m.subSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.invocSvc.EXPECT().DeleteByCustomer(mock.Anything, c).Return(nil) + m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-1"}). + Return([]checkout.Checkout{}, nil) + m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-1").Return(nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-1"). + Return(0, errors.New("balance check failed")) + // strict mocks: creditSvc.DeleteByAccountID and custSvc.Delete must not be called + + err := m.build().DeleteCustomers(context.Background(), "org-1") + assert.ErrorContains(t, err, "balance check failed") + }) + t.Run("offline account still removes local billing records", func(t *testing.T) { m := newMocks(t) @@ -309,6 +586,7 @@ func TestDeleteCustomers(t *testing.T) { m.checkoutSvc.EXPECT().List(mock.Anything, checkout.Filter{CustomerID: "cust-no-provider"}). Return([]checkout.Checkout{}, nil) m.checkoutSvc.EXPECT().DeleteByCustomer(mock.Anything, "cust-no-provider").Return(nil) + m.creditSvc.EXPECT().GetBalance(mock.Anything, "cust-no-provider").Return(0, nil) m.creditSvc.EXPECT().DeleteByAccountID(mock.Anything, "cust-no-provider").Return(nil) m.custSvc.EXPECT().Delete(mock.Anything, "cust-no-provider").Return(nil) diff --git a/go.mod b/go.mod index 6c822de1a9..413d9e8673 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( golang.org/x/oauth2 v0.36.0 golang.org/x/sync v0.22.0 google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 + google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 google.golang.org/grpc v1.82.1 google.golang.org/protobuf v1.36.11 gopkg.in/dnaeon/go-vcr.v3 v3.1.2 @@ -133,7 +134,6 @@ require ( golang.org/x/time v0.5.0 // indirect golang.org/x/tools v0.47.0 // indirect google.golang.org/genproto v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/square/go-jose.v2 v2.6.0 // indirect ) diff --git a/internal/api/v1beta1connect/deleter.go b/internal/api/v1beta1connect/deleter.go index 6d14cfbb3f..4cf66c6b35 100644 --- a/internal/api/v1beta1connect/deleter.go +++ b/internal/api/v1beta1connect/deleter.go @@ -2,10 +2,15 @@ package v1beta1connect import ( "context" + "errors" "fmt" + "log/slog" "connectrpc.com/connect" + "github.com/raystack/frontier/core/deleter" + "github.com/raystack/frontier/core/organization" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" + "google.golang.org/genproto/googleapis/rpc/errdetails" ) func (h *ConnectHandler) DeleteProject(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteProjectRequest]) (*connect.Response[frontierv1beta1.DeleteProjectResponse], error) { @@ -17,7 +22,36 @@ func (h *ConnectHandler) DeleteProject(ctx context.Context, request *connect.Req func (h *ConnectHandler) DeleteOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteOrganizationRequest]) (*connect.Response[frontierv1beta1.DeleteOrganizationResponse], error) { if err := h.deleterService.DeleteOrganization(ctx, request.Msg.GetId()); err != nil { + var blocked *deleter.BlockedError + if errors.As(err, &blocked) { + return nil, deleteBlockedError(ctx, blocked) + } + if errors.Is(err, organization.ErrNotExist) || errors.Is(err, organization.ErrInvalidUUID) || errors.Is(err, organization.ErrInvalidID) { + return nil, connect.NewError(connect.CodeNotFound, organization.ErrNotExist) + } return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteOrganization.DeleteOrganization: organization_id=%s: %w", request.Msg.GetId(), err)) } return connect.NewResponse(&frontierv1beta1.DeleteOrganizationResponse{}), nil } + +// deleteBlockedError turns the delete's blockers into a failed_precondition +// error carrying one PreconditionFailure violation per blocker, so a caller +// sees everything to fix in a single response. +func deleteBlockedError(ctx context.Context, blocked *deleter.BlockedError) *connect.Error { + connectErr := connect.NewError(connect.CodeFailedPrecondition, blocked) + failure := &errdetails.PreconditionFailure{} + for _, b := range blocked.Blockers { + failure.Violations = append(failure.Violations, &errdetails.PreconditionFailure_Violation{ + Type: b.Type, + Subject: b.Subject, + Description: b.Message, + }) + } + if detail, err := connect.NewErrorDetail(failure); err != nil { + slog.WarnContext(ctx, "failed to attach precondition failure details", "error", err, "org_id", blocked.OrgID) + } else { + connectErr.AddDetail(detail) + } + slog.WarnContext(ctx, "organization delete blocked", "org_id", blocked.OrgID, "blockers", len(blocked.Blockers)) + return connectErr +} diff --git a/internal/api/v1beta1connect/deleter_test.go b/internal/api/v1beta1connect/deleter_test.go index 351769c9e7..3898fbf5c7 100644 --- a/internal/api/v1beta1connect/deleter_test.go +++ b/internal/api/v1beta1connect/deleter_test.go @@ -6,11 +6,14 @@ import ( "testing" "connectrpc.com/connect" + "github.com/raystack/frontier/core/deleter" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/internal/api/v1beta1connect/mocks" "github.com/raystack/frontier/pkg/errors" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/genproto/googleapis/rpc/errdetails" ) func TestHandler_DeleteProject(t *testing.T) { @@ -77,6 +80,17 @@ func TestHandler_DeleteOrganization(t *testing.T) { want: connect.NewResponse(&frontierv1beta1.DeleteOrganizationResponse{}), wantErr: nil, }, + { + name: "should return not found when the org is already gone", + setup: func(as *mocks.CascadeDeleter) { + as.EXPECT().DeleteOrganization(mock.Anything, "some-id").Return(organization.ErrNotExist) + }, + request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{ + Id: "some-id", + }), + want: nil, + wantErr: connect.NewError(connect.CodeNotFound, organization.ErrNotExist), + }, { name: "should return error if deleter service encounters an error", setup: func(as *mocks.CascadeDeleter) { @@ -101,4 +115,39 @@ func TestHandler_DeleteOrganization(t *testing.T) { assert.Equal(t, tt.wantErr, err) }) } + + t.Run("should return failed precondition with one violation per blocker when the delete is blocked", func(t *testing.T) { + blocked := &deleter.BlockedError{ + OrgID: "some-id", + Blockers: []deleter.Blocker{ + {Type: deleter.BlockerUnpaidInvoice, Subject: "inv-1", Message: "invoice[inv-1] is unpaid: pay it via its hosted payment page, then retry the delete"}, + {Type: deleter.BlockerNegativeTokenBalance, Subject: "cust-1", Message: "billing account[cust-1] owes 50 tokens: contact support to settle the balance, then retry the delete"}, + }, + } + mockDelOrg := new(mocks.CascadeDeleter) + mockDelOrg.EXPECT().DeleteOrganization(mock.Anything, "some-id").Return(blocked) + mockDep := &ConnectHandler{deleterService: mockDelOrg} + + resp, err := mockDep.DeleteOrganization(context.Background(), connect.NewRequest(&frontierv1beta1.DeleteOrganizationRequest{ + Id: "some-id", + })) + assert.Nil(t, resp) + + var connectErr *connect.Error + assert.ErrorAs(t, err, &connectErr) + assert.Equal(t, connect.CodeFailedPrecondition, connectErr.Code()) + assert.Contains(t, connectErr.Message(), "pay it") + assert.Contains(t, connectErr.Message(), "contact support") + + assert.Len(t, connectErr.Details(), 1) + detail, detailErr := connectErr.Details()[0].Value() + assert.NoError(t, detailErr) + failure, ok := detail.(*errdetails.PreconditionFailure) + assert.True(t, ok) + assert.Len(t, failure.GetViolations(), 2) + assert.Equal(t, deleter.BlockerUnpaidInvoice, failure.GetViolations()[0].GetType()) + assert.Equal(t, "inv-1", failure.GetViolations()[0].GetSubject()) + assert.Equal(t, deleter.BlockerNegativeTokenBalance, failure.GetViolations()[1].GetType()) + assert.Equal(t, "cust-1", failure.GetViolations()[1].GetSubject()) + }) }