Cancel Stripe subscription when an organization is deleted

lib/api_organizations/src/organizations.rs

@@ -25,6 +25,9 @@ use dropshot::{HttpError, Path, Query, RequestContext, TypedBody, endpoint};
 use schemars::JsonSchema;
 use serde::Deserialize;
 
+#[cfg(feature = "plus")]
+use crate::plan::cancel_and_remove_plan;
+
 pub type OrganizationsPagination = JsonPagination<OrganizationsSort>;
 
 #[derive(Debug, Clone, Copy, Default, Deserialize, JsonSchema)]
@@ -334,6 +337,7 @@ async fn patch_inner(
 /// The user must have `delete` permissions for the organization.
 /// By default, organizations are soft-deleted (along with their child projects).
 /// Set the `hard` query parameter to `true` to permanently delete the organization (requires server admin).
+/// On Bencher Cloud, deleting an organization also immediately cancels any active subscription.
 #[endpoint {
     method = DELETE,
     path =  "/v0/organizations/{organization}",
@@ -384,6 +388,12 @@ async fn delete_inner(
                 Organization,
                 &path_params.organization
             ))?;
+
+        // Cancel any active Stripe subscription and remove the plan row before deleting the
+        // organization so no dangling subscription is left in Stripe.
+        #[cfg(feature = "plus")]
+        cancel_and_remove_plan(context, &query_organization).await?;
+
         diesel::delete(
             schema::organization::table.filter(schema::organization::id.eq(query_organization.id)),
         )
@@ -399,6 +409,11 @@ async fn delete_inner(
             Permission::Delete,
         )?;
 
+        // Cancel any active Stripe subscription and remove the plan row before deleting the
+        // organization so no dangling subscription is left in Stripe.
+        #[cfg(feature = "plus")]
+        cancel_and_remove_plan(context, &query_organization).await?;
+
         // Soft delete the organization and all its child projects
         let now = context.clock.now();
         query_organization.soft_delete(context, log, now).await?;

lib/api_organizations/src/plan.rs

@@ -5,7 +5,7 @@ use bencher_endpoint::{
     CorsResponse, Delete, Endpoint, Get, Patch, Post, ResponseCreated, ResponseDeleted, ResponseOk,
 };
 use bencher_json::{
-    DateTime, OrganizationResourceId, PlanStatus,
+    OrganizationResourceId, PlanStatus,
     organization::plan::{JsonNewPlan, JsonPlan, JsonUpdatePlan},
 };
 use bencher_rbac::organization::Permission;
@@ -28,7 +28,10 @@ use bencher_schema::{
     },
     schema, write_conn,
 };
-use diesel::{BelongingToDsl as _, ExpressionMethods as _, QueryDsl as _, RunQueryDsl as _};
+use diesel::{
+    BelongingToDsl as _, ExpressionMethods as _, OptionalExtension as _, QueryDsl as _,
+    RunQueryDsl as _,
+};
 use dropshot::{HttpError, Path, Query, RequestContext, TypedBody, endpoint};
 use schemars::JsonSchema;
 use serde::Deserialize;
@@ -350,14 +353,16 @@ async fn prune_or_conflict_existing_plan(
         .await
         .map_err(service_unavailable_error)?;
 
-    match existing_plan_action(status) {
-        ExistingPlan::Conflict => Err(plan_conflict(query_organization, query_plan)),
-        ExistingPlan::Prune => {
-            diesel::delete(schema::plan::table.filter(schema::plan::id.eq(query_plan.id)))
-                .execute(write_conn!(context))
-                .map_err(resource_conflict_err!(Plan, query_plan))?;
-            Ok(())
-        },
+    if subscription_is_live(status) {
+        // Still live in Stripe (active, trialing, or dunning): block, so we never orphan a
+        // live subscription and let the org create a duplicate.
+        Err(plan_conflict(query_organization, query_plan))
+    } else {
+        // Gone or terminal: prune the stale row so the org can subscribe again.
+        diesel::delete(schema::plan::table.filter(schema::plan::id.eq(query_plan.id)))
+            .execute(write_conn!(context))
+            .map_err(resource_conflict_err!(Plan, query_plan))?;
+        Ok(())
     }
 }
 
@@ -371,31 +376,29 @@ fn plan_conflict(query_organization: &QueryOrganization, query_plan: QueryPlan)
     )
 }
 
-/// Whether an organization's existing metered plan row blocks creating a new plan, or is
-/// a stale row to prune. `status` is `None` when Stripe reports the subscription gone
-/// (404), else its live status.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-enum ExistingPlan {
-    Conflict,
-    Prune,
-}
-
-/// Prune (allow re-subscribe) only when the subscription is gone or *terminal* (canceled
-/// / incomplete-expired). An active, trialing, or recoverable (dunning) subscription
-/// still exists in Stripe and must block, so we never orphan a live subscription and let
-/// the org create a duplicate. Matched exhaustively so a new `PlanStatus` forces a
-/// decision here.
-fn existing_plan_action(status: Option<PlanStatus>) -> ExistingPlan {
+/// Whether a subscription is still live in Stripe, given its status (`None` when Stripe
+/// reports it gone via a 404).
+///
+/// Live means active, trialing, or in a recoverable dunning state (`past_due` / `unpaid` /
+/// `incomplete` / `paused`): the subscription still exists in Stripe. Not live means gone
+/// (404) or *terminal* (canceled / incomplete-expired). Matched exhaustively so a new
+/// `PlanStatus` forces a decision here.
+///
+/// This answers the single question behind two separate decisions: plan creation blocks on
+/// a live subscription (otherwise it prunes the stale row so the org can re-subscribe), and
+/// plan deletion cancels a live subscription (otherwise it skips, since canceling a
+/// gone/terminal one would 404).
+fn subscription_is_live(status: Option<PlanStatus>) -> bool {
     match status {
-        None | Some(PlanStatus::Canceled | PlanStatus::IncompleteExpired) => ExistingPlan::Prune,
+        None | Some(PlanStatus::Canceled | PlanStatus::IncompleteExpired) => false,
         Some(
             PlanStatus::Active
             | PlanStatus::Trialing
             | PlanStatus::PastDue
             | PlanStatus::Unpaid
             | PlanStatus::Incomplete
             | PlanStatus::Paused,
-        ) => ExistingPlan::Conflict,
+        ) => true,
     }
 }
 
@@ -470,6 +473,46 @@ async fn delete_inner(
     delete_plan_result
 }
 
+/// Cancel an organization's live Stripe subscription (if any) and remove its local plan
+/// row. Called from organization deletion so no dangling subscription is left in Stripe
+/// when an organization is deleted. A no-op when the server has no biller (Self-Hosted) or
+/// the organization has no plan row.
+///
+/// The Stripe cancel cannot share a database transaction with the caller's organization
+/// delete, so there is a small non-atomic window. Cancel-first ordering keeps it on the safe
+/// side: if the org delete then fails, the org is briefly present with its subscription
+/// already canceled (recoverable on retry) rather than deleted with a live subscription.
+pub(crate) async fn cancel_and_remove_plan(
+    context: &ApiContext,
+    query_organization: &QueryOrganization,
+) -> Result<(), HttpError> {
+    // Only Bencher Cloud has a biller and plan rows; nothing to cancel otherwise.
+    let Ok(biller) = context.biller() else {
+        return Ok(());
+    };
+    // A missing plan row means nothing to cancel, but a real database error must not be
+    // mistaken for "no plan" (that would delete the org while its subscription is still
+    // live). `.optional()` maps only `NotFound` to `None` and propagates every other error.
+    let Some(query_plan) = QueryPlan::belonging_to(query_organization)
+        .first::<QueryPlan>(auth_conn!(context))
+        .optional()
+        .map_err(resource_conflict_err!(Plan, query_organization))?
+    else {
+        return Ok(());
+    };
+
+    // Cancel in Stripe first; propagate the error (aborting the organization deletion) if it
+    // fails so we never delete the organization while its subscription is still live.
+    delete_plan(context, biller, query_organization, &query_plan, true).await?;
+
+    // The subscription is canceled (or was already gone); remove the local plan row.
+    diesel::delete(schema::plan::table.filter(schema::plan::id.eq(query_plan.id)))
+        .execute(write_conn!(context))
+        .map_err(resource_conflict_err!(Plan, query_plan))?;
+
+    Ok(())
+}
+
 async fn delete_plan(
     context: &ApiContext,
     biller: &Biller,
@@ -479,17 +522,35 @@ async fn delete_plan(
 ) -> Result<(), HttpError> {
     if let Some(metered_plan_id) = query_plan.metered_plan.as_ref() {
         if remote {
-            biller
-                .cancel_metered_subscription(metered_plan_id)
+            // Only cancel a subscription still live in Stripe; a gone/terminal one is
+            // already effectively canceled, so canceling it again would 404. A transient
+            // Stripe error surfaces as 503 rather than being mistaken for "gone".
+            let status = biller
+                .metered_plan_status(metered_plan_id)
                 .await
-                .map_err(resource_not_found_err!(Plan, query_plan))?;
+                .map_err(service_unavailable_error)?;
+            if subscription_is_live(status) {
+                biller
+                    .cancel_metered_subscription(metered_plan_id)
+                    .await
+                    .map_err(resource_not_found_err!(Plan, query_plan))?;
+            }
         }
     } else if let Some(licensed_plan_id) = query_plan.licensed_plan.as_ref() {
         if remote {
-            biller
-                .cancel_licensed_subscription(licensed_plan_id)
+            // Only cancel a subscription still live in Stripe; a gone/terminal one is
+            // already effectively canceled, so canceling it again would 404. A transient
+            // Stripe error surfaces as 503 rather than being mistaken for "gone".
+            let status = biller
+                .licensed_plan_status(licensed_plan_id)
                 .await
-                .map_err(resource_not_found_err!(Plan, query_plan))?;
+                .map_err(service_unavailable_error)?;
+            if subscription_is_live(status) {
+                biller
+                    .cancel_licensed_subscription(licensed_plan_id)
+                    .await
+                    .map_err(resource_not_found_err!(Plan, query_plan))?;
+            }
         }
 
         if query_organization.license.is_some() {
@@ -499,7 +560,7 @@ async fn delete_plan(
                 name: None,
                 slug: None,
                 license: Some(None),
-                modified: DateTime::now(),
+                modified: context.clock.now(),
             };
             diesel::update(organization_query)
                 .set(&update_organization)
@@ -523,22 +584,18 @@ async fn delete_plan(
 mod tests {
     use bencher_json::PlanStatus;
 
-    use super::{ExistingPlan, existing_plan_action};
+    use super::subscription_is_live;
 
     #[test]
-    fn existing_plan_action_decides() {
-        // Gone in Stripe (404) or terminal: prune the stale row.
-        assert_eq!(existing_plan_action(None), ExistingPlan::Prune);
-        assert_eq!(
-            existing_plan_action(Some(PlanStatus::Canceled)),
-            ExistingPlan::Prune,
-        );
-        assert_eq!(
-            existing_plan_action(Some(PlanStatus::IncompleteExpired)),
-            ExistingPlan::Prune,
-        );
-        // Active, trialing, or recoverable (dunning) still exists in Stripe: block, so we
-        // never orphan a live subscription and let the org create a duplicate.
+    fn subscription_is_live_decides() {
+        // Gone in Stripe (404) or terminal: not live, so plan creation prunes the stale row
+        // and plan deletion skips canceling.
+        assert!(!subscription_is_live(None));
+        assert!(!subscription_is_live(Some(PlanStatus::Canceled)));
+        assert!(!subscription_is_live(Some(PlanStatus::IncompleteExpired)));
+        // Active, trialing, or recoverable (dunning) still exists in Stripe: live, so plan
+        // creation blocks (never orphan a live subscription and let the org create a
+        // duplicate) and plan deletion cancels.
         for status in [
             PlanStatus::Active,
             PlanStatus::Trialing,
@@ -547,7 +604,7 @@ mod tests {
             PlanStatus::Incomplete,
             PlanStatus::Paused,
         ] {
-            assert_eq!(existing_plan_action(Some(status)), ExistingPlan::Conflict);
+            assert!(subscription_is_live(Some(status)));
         }
     }
 }

plus/bencher_billing/src/biller.rs

@@ -836,18 +836,41 @@ impl Biller {
         })
     }
 
-    /// Resolve a metered subscription's live status for re-subscription gating:
-    /// `Some(status)` when the subscription exists, and `None` when Stripe reports it no
-    /// longer exists (a definitive 404, safe to treat as gone). Returns an error only for
-    /// indeterminate failures (network, 5xx, rate limit), so a transient outage is never
-    /// mistaken for "gone" (which would risk pruning a still-live subscription). The
+    /// Resolve a metered subscription's live status for the two decisions keyed on whether
+    /// it is still live in Stripe (re-subscription gating on plan creation, and
+    /// cancel-vs-skip on plan deletion): `Some(status)` when the subscription exists, and
+    /// `None` when Stripe reports it no longer exists (a definitive 404, safe to treat as
+    /// gone). Returns an error only for indeterminate failures (network, 5xx, rate limit),
+    /// so a transient outage is never mistaken for "gone" (which would risk orphaning a
+    /// still-live subscription: pruned on creation, or left uncanceled on deletion). The
     /// caller distinguishes terminal from recoverable (dunning) statuses.
     pub async fn metered_plan_status(
         &self,
         metered_plan_id: &MeteredPlanId,
     ) -> Result<Option<PlanStatus>, BillingError> {
         let subscription_id: SubscriptionId = metered_plan_id.as_ref().into();
-        match self.get_subscription(&subscription_id).await {
+        self.subscription_status(&subscription_id).await
+    }
+
+    /// The live status of a licensed subscription, or `None` when Stripe reports it gone
+    /// (404). Mirrors [`Self::metered_plan_status`] so callers can tell a still-live
+    /// subscription from one that is already gone or terminal. Unlike
+    /// [`Self::get_licensed_plan_status`], a 404 is reported as `None` rather than an error.
+    pub async fn licensed_plan_status(
+        &self,
+        licensed_plan_id: &LicensedPlanId,
+    ) -> Result<Option<PlanStatus>, BillingError> {
+        let subscription_id: SubscriptionId = licensed_plan_id.as_ref().into();
+        self.subscription_status(&subscription_id).await
+    }
+
+    /// The live status of a subscription, or `None` when Stripe reports it gone (404). A
+    /// transient Stripe error is returned as-is so callers can distinguish it from "gone".
+    async fn subscription_status(
+        &self,
+        subscription_id: &SubscriptionId,
+    ) -> Result<Option<PlanStatus>, BillingError> {
+        match self.get_subscription(subscription_id).await {
             Ok(subscription) => Ok(Some(Self::map_status(&subscription.status))),
             Err(BillingError::Stripe(stripe::StripeError::Stripe(_, 404))) => Ok(None),
             Err(e) => Err(e),
@@ -868,6 +891,9 @@ impl Biller {
         )
     }
 
+    /// The status of a licensed subscription, erroring if Stripe reports it gone (404). For
+    /// callers that must treat a gone subscription as absent rather than an error, see
+    /// [`Self::licensed_plan_status`].
     pub async fn get_licensed_plan_status(
         &self,
         licensed_plan_id: &LicensedPlanId,
@@ -1063,6 +1089,55 @@ mod tests {
         assert_eq!(plan_level_from_pro_price(false), PlanLevel::Team);
     }
 
+    #[test]
+    fn map_status_maps_subscription_status() {
+        use stripe_billing::SubscriptionStatus;
+
+        // Each Stripe subscription status maps to the matching plan status. This mapping
+        // feeds `metered_plan_status`/`licensed_plan_status`, which drive the
+        // cancel-vs-skip decision in `subscription_is_live`, so a wrong mapping (e.g. a
+        // terminal status reading as `Active`) would cancel or skip incorrectly.
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Active),
+            PlanStatus::Active,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Canceled),
+            PlanStatus::Canceled,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Incomplete),
+            PlanStatus::Incomplete,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::IncompleteExpired),
+            PlanStatus::IncompleteExpired,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::PastDue),
+            PlanStatus::PastDue,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Paused),
+            PlanStatus::Paused,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Trialing),
+            PlanStatus::Trialing,
+        );
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Unpaid),
+            PlanStatus::Unpaid,
+        );
+        // An unrecognized/future Stripe status falls through the wildcard arm to `Unpaid`.
+        // That is the safe side for deletion: `Unpaid` reads as live, so a cancel is
+        // attempted rather than silently skipped and left to orphan.
+        assert_eq!(
+            Biller::map_status(&SubscriptionStatus::Unknown("future_status".to_owned())),
+            PlanStatus::Unpaid,
+        );
+    }
+
     #[test]
     fn get_plan_unit_amount_reads_price() {
         let mut item = make_subscription_item("price_metrics");

services/api/openapi.json

@@ -1172,7 +1172,7 @@
           "organizations"
         ],
         "summary": "Delete an organization",
-        "description": "Delete an organization where the user is a member. The user must have `delete` permissions for the organization. By default, organizations are soft-deleted (along with their child projects). Set the `hard` query parameter to `true` to permanently delete the organization (requires server admin).",
+        "description": "Delete an organization where the user is a member. The user must have `delete` permissions for the organization. By default, organizations are soft-deleted (along with their child projects). Set the `hard` query parameter to `true` to permanently delete the organization (requires server admin). On Bencher Cloud, deleting an organization also immediately cancels any active subscription.",
         "operationId": "organization_delete",
         "parameters": [
           {