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()) + }) }