From 7117b6af006f44228ab36aa396ae47de18ac02a5 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 | 40 ++- core/deleter/service.go | 205 ++++++++++++- 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, 606 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 0ada531eea..a092b9c600 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -581,7 +581,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..d1f42a25d5 100644 --- a/core/deleter/deleter.go +++ b/core/deleter/deleter.go @@ -1,7 +1,41 @@ package deleter -import "fmt" +import "strings" -var ( - ErrDeleteNotAllowed = fmt.Errorf("deletion not allowed for billed accounts") +// Blocker types returned by the org delete pre-flight check. They are +// machine-readable and end up as PreconditionFailure violation types on +// the API error, so clients can branch on them. +const ( + BlockerActiveSubscription = "ACTIVE_SUBSCRIPTION" + BlockerUnpaidInvoice = "UNPAID_INVOICE" + BlockerNegativeTokenBalance = "NEGATIVE_TOKEN_BALANCE" ) + +// Blocker is one reason an organization cannot be deleted right now. The +// message names the fix: downgrading a paid subscription and paying an +// invoice are things the caller can do themselves, settling a token debt +// goes through support. +type Blocker struct { + // Type is one of the Blocker* constants. + Type string + // Subject is the id of the blocking entity, e.g. a subscription id. + Subject string + // Message says what blocks the delete and what to do about it. + Message string +} + +// BlockedError carries every blocker the pre-flight 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..8d00300d35 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,11 @@ type Service struct { checkoutService CheckoutService creditService CreditService kycService KycService + planService PlanService + // defaultPlan names the plan a subscription may still be on when the org + // is deleted; subscriptions on any other plan block the delete until the + // caller downgrades them + defaultPlan string } func NewCascadeDeleter(orgService OrganizationService, projService ProjectService, @@ -148,7 +163,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 +183,8 @@ func NewCascadeDeleter(orgService OrganizationService, projService ProjectServic checkoutService: checkoutService, creditService: creditService, kycService: kycService, + planService: planService, + defaultPlan: defaultPlan, } } @@ -217,9 +235,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.preflight(ctx, id); err != nil { + return err } // delete all billing accounts @@ -364,6 +389,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 +453,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 +// preflight 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 caller downgrades it to +// the standard (default) plan first. A subscription on the standard plan +// does not block — once nothing else does, preflight cancels it itself, +// before any deletion starts. The cancel is immediate and invoices unbilled +// usage on the spot, so the invoice check runs again after it — a final +// invoice coming out of the cancellation 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) preflight(ctx context.Context, id string) error { customers, err := d.customerService.List(ctx, customer.Filter{ OrgID: id, }) @@ -421,14 +480,130 @@ 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) + } + 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} + } + + // nothing blocks the delete; the only subscriptions still running are on + // the standard plan — cancel them and re-check the invoices the cancel + // may have created. This happens only once every blocker is clear, so a + // delete that stays blocked does not cost 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..959702fef6 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 pre-flight 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 pre-flight + + 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..07b9dd329d 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 pre-flight 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()) + }) }