corporate: Make initial upgrade license checks more robust.

In cases where the seat count for automated license management is
stale when the upgrade process is initiated and the user has chosen
automated license management, we should get the current billable
license count when doing the initial payment/charge.

Also, makes the post-payment check for inconsistencies more robust
in that we check for both under and over billing cases. In the case
where the customer may have been overbilled, an email is sent to
the billing support team so that manual investigation can happen.

corporate/lib/stripe.py

@@ -1704,6 +1704,15 @@ class BillingSession(ABC):
         )
         return self.create_stripe_invoice_and_charge(updated_metadata)
 
+    def stale_seat_count_check(self, request_seat_count: int, tier: int) -> int:
+        current_seat_count = self.current_count_for_billed_licenses()
+        minimum_seat_count = self.min_licenses_for_plan(tier)
+        if request_seat_count == minimum_seat_count and current_seat_count < minimum_seat_count:
+            # Continue to use the minimum licenses for the plan tier.
+            return request_seat_count
+        # Otherwise, use a current count for billed licenses.
+        return current_seat_count
+
     def ensure_current_plan_is_upgradable(self, customer: Customer, new_plan_tier: int) -> None:
         # Upgrade for customers with an existing plan is only supported for remote realm / server right now.
         if isinstance(self, RealmBillingSession):
@@ -1785,13 +1794,18 @@ class BillingSession(ABC):
         # TODO: The correctness of this relies on user creation, deactivation, etc being
         # in a transaction.atomic() with the relevant RealmAuditLog entries
         with transaction.atomic():
-            # billed_licenses can be greater than licenses if users are added between the start of
-            # this function (process_initial_upgrade) and now
+            # We get the current license count here in case the number of billable
+            # licenses has changed since the upgrade process began.
             current_licenses_count = self.get_billable_licenses_for_customer(
                 customer, plan_tier, licenses
             )
-            # In case user wants more licenses for the plan. (manual license management)
-            billed_licenses = max(current_licenses_count, licenses)
+            if current_licenses_count != licenses and not automanage_licenses:
+                # With manual license management, the user may want to purchase more
+                # licenses than are currently in use.
+                billable_licenses = max(current_licenses_count, licenses)
+            else:
+                billable_licenses = current_licenses_count
+
             plan_params = {
                 "automanage_licenses": automanage_licenses,
                 "charge_automatically": charge_automatically,
@@ -1862,7 +1876,7 @@ class BillingSession(ABC):
                 )
                 # Update license_at_next_renewal as per new plan.
                 assert last_ledger_entry is not None
-                last_ledger_entry.licenses_at_next_renewal = billed_licenses
+                last_ledger_entry.licenses_at_next_renewal = billable_licenses
                 last_ledger_entry.save(update_fields=["licenses_at_next_renewal"])
                 remote_server_legacy_plan.status = CustomerPlan.SWITCH_PLAN_TIER_AT_PLAN_END
                 remote_server_legacy_plan.save(update_fields=["status"])
@@ -1910,21 +1924,47 @@ class BillingSession(ABC):
                 # TODO: Do a check for max licenses for fixed price plans here after we add that.
                 if (
                     stripe_invoice_paid
-                    and billed_licenses != licenses
+                    and billable_licenses != licenses
                     and not customer.exempt_from_license_number_check
                     and not fixed_price_plan_offer
                 ):
-                    # Customer paid for less licenses than they have.
+                    # Billable licenses in use do not match what was paid/invoiced by customer.
+                    if billable_licenses > licenses:
+                        # Customer paid for less licenses than they have in use.
                         # We need to create a new ledger entry to track the additional licenses.
                         LicenseLedger.objects.create(
                             plan=plan,
                             is_renewal=False,
                             event_time=event_time,
-                        licenses=billed_licenses,
-                        licenses_at_next_renewal=billed_licenses,
+                            licenses=billable_licenses,
+                            licenses_at_next_renewal=billable_licenses,
                         )
                         # Creates due today invoice for additional licenses.
                         self.invoice_plan(plan, event_time)
+                    else:
+                        # Customer paid for more licenses than they have in use.
+                        # We need to create a new ledger entry to track the reduced renewal licenses.
+                        LicenseLedger.objects.create(
+                            plan=plan,
+                            is_renewal=False,
+                            event_time=event_time,
+                            licenses=licenses,
+                            licenses_at_next_renewal=billable_licenses,
+                        )
+                        # Send internal billing notice about license discrepancy.
+                        context = {
+                            "billing_entity": self.billing_entity_display_name,
+                            "support_url": self.support_url(),
+                            "paid_licenses": licenses,
+                            "current_licenses": billable_licenses,
+                            "notice_reason": "license_discrepancy",
+                        }
+                        send_email(
+                            "zerver/emails/internal_billing_notice",
+                            to_emails=[BILLING_SUPPORT_EMAIL],
+                            from_address=FromAddress.tokenized_no_reply_address(),
+                            context=context,
+                        )
 
         if not stripe_invoice_paid and not (
             free_trial or should_schedule_upgrade_for_legacy_remote_server
@@ -1936,7 +1976,7 @@ class BillingSession(ABC):
                 customer,
                 price_per_license=price_per_license,
                 fixed_price=plan.fixed_price,
-                licenses=billed_licenses,
+                licenses=billable_licenses,
                 plan_tier=plan.tier,
                 billing_schedule=billing_schedule,
                 charge_automatically=False,
@@ -1953,7 +1993,7 @@ class BillingSession(ABC):
             # fails to pay the invoice before expiration, we downgrade the customer.
             self.generate_stripe_invoice(
                 plan_tier,
-                licenses=billed_licenses,
+                licenses=billable_licenses,
                 license_management="automatic" if automanage_licenses else "manual",
                 billing_schedule=billing_schedule,
                 billing_modality="send_invoice",
@@ -1968,15 +2008,21 @@ class BillingSession(ABC):
             self.ensure_current_plan_is_upgradable(customer, upgrade_request.tier)
         billing_modality = upgrade_request.billing_modality
         schedule = upgrade_request.schedule
-        license_management = upgrade_request.license_management
-        licenses = upgrade_request.licenses
 
-        seat_count = unsign_seat_count(upgrade_request.signed_seat_count, upgrade_request.salt)
-        if billing_modality == "charge_automatically" and license_management == "automatic":
-            licenses = seat_count
+        license_management = upgrade_request.license_management
         if billing_modality == "send_invoice":
             license_management = "manual"
 
+        licenses = upgrade_request.licenses
+        request_seat_count = unsign_seat_count(
+            upgrade_request.signed_seat_count, upgrade_request.salt
+        )
+        # For automated license management, we check for changes to the
+        # billable licenses count made after the billing portal was loaded.
+        seat_count = self.stale_seat_count_check(request_seat_count, upgrade_request.tier)
+        if billing_modality == "charge_automatically" and license_management == "automatic":
+            licenses = seat_count
+
         exempt_from_license_number_check = (
             customer is not None and customer.exempt_from_license_number_check
         )

corporate/tests/test_stripe.py

@@ -1962,6 +1962,7 @@ class StripeTest(StripeTestCase):
     def test_upgrade_by_card_with_outdated_seat_count(self, *mocks: Mock) -> None:
         hamlet = self.example_user("hamlet")
         self.login_user(hamlet)
+        # Higher than original seat count
         new_seat_count = 23
         initial_upgrade_request = InitialUpgradeRequest(
             manual_license_management=False,
@@ -1972,8 +1973,12 @@ class StripeTest(StripeTestCase):
         _, context_when_upgrade_page_is_rendered = billing_session.get_initial_upgrade_context(
             initial_upgrade_request
         )
-        # Change the seat count while the user is going through the upgrade flow
+        # Change the seat count in upgrade flow: after do_upgrade, during process_initial_upgrade
         with (
+            patch(
+                "corporate.lib.stripe.BillingSession.stale_seat_count_check",
+                return_value=self.seat_count,
+            ),
             patch("corporate.lib.stripe.get_latest_seat_count", return_value=new_seat_count),
             patch(
                 "corporate.lib.stripe.RealmBillingSession.get_initial_upgrade_context",
@@ -2006,6 +2011,65 @@ class StripeTest(StripeTestCase):
         self.assertEqual(ledger_entry.licenses, new_seat_count)
         self.assertEqual(ledger_entry.licenses_at_next_renewal, new_seat_count)
 
+    @mock_stripe()
+    def test_upgrade_by_card_with_outdated_lower_seat_count(self, *mocks: Mock) -> None:
+        hamlet = self.example_user("hamlet")
+        self.login_user(hamlet)
+        new_seat_count = self.seat_count - 1
+        initial_upgrade_request = InitialUpgradeRequest(
+            manual_license_management=False,
+            tier=CustomerPlan.TIER_CLOUD_STANDARD,
+            billing_modality="charge_automatically",
+        )
+        billing_session = RealmBillingSession(hamlet)
+        _, context_when_upgrade_page_is_rendered = billing_session.get_initial_upgrade_context(
+            initial_upgrade_request
+        )
+        # Change the seat count in upgrade flow: after do_upgrade, during process_initial_upgrade
+        with (
+            patch(
+                "corporate.lib.stripe.BillingSession.stale_seat_count_check",
+                return_value=self.seat_count,
+            ),
+            patch("corporate.lib.stripe.get_latest_seat_count", return_value=new_seat_count),
+            patch(
+                "corporate.lib.stripe.RealmBillingSession.get_initial_upgrade_context",
+                return_value=(_, context_when_upgrade_page_is_rendered),
+            ),
+        ):
+            self.add_card_and_upgrade(hamlet)
+
+        customer = Customer.objects.first()
+        assert customer is not None
+        stripe_customer_id: str = assert_is_not_none(customer.stripe_customer_id)
+        # Check that the Charge used the old quantity, not new_seat_count
+        [charge] = iter(stripe.Charge.list(customer=stripe_customer_id))
+        self.assertEqual(8000 * self.seat_count, charge.amount)
+        [upgrade_invoice] = iter(stripe.Invoice.list(customer=stripe_customer_id))
+        self.assertEqual(
+            [8000 * self.seat_count],
+            [item.amount for item in upgrade_invoice.lines],
+        )
+        # Check LicenseLedger has the reduced license count at renewal
+        ledger_entry = LicenseLedger.objects.last()
+        assert ledger_entry is not None
+        self.assertEqual(ledger_entry.licenses, self.seat_count)
+        self.assertEqual(ledger_entry.licenses_at_next_renewal, new_seat_count)
+
+        # Check that we informed the support team about the potential billing error.
+        from django.core.mail import outbox
+
+        self.assert_length(outbox, 1)
+
+        for message in outbox:
+            self.assert_length(message.to, 1)
+            self.assertEqual(message.to[0], "sales@zulip.com")
+            self.assertEqual(
+                message.subject,
+                f"Check initial licenses invoiced for {billing_session.billing_entity_display_name}",
+            )
+            self.assertEqual(self.email_envelope_from(message), settings.NOREPLY_EMAIL_ADDRESS)
+
     def test_upgrade_with_tampered_seat_count(self) -> None:
         hamlet = self.example_user("hamlet")
         self.login_user(hamlet)

templates/zerver/emails/internal_billing_notice.html

@@ -11,6 +11,10 @@
 <b>Last data upload</b>: {{ last_audit_log_update }}
 {% elif notice_reason == "locally_deleted_realm_on_paid_plan" %}
 <p>Investigate why remote realm is marked as locally deleted when it's on a paid plan.</p>
+{% elif notice_reason == "license_discrepancy" %}
+<p>Discrepancy in licenses when upgraded to current plan.</p>
+<b>Licenses paid for</b>: {{ paid_licenses }}.
+<b>Reported licenses in use</b>: {{ current_licenses }}.
 {% endif %}
 
 <br /><br />

templates/zerver/emails/internal_billing_notice.subject.txt

@@ -4,4 +4,6 @@ Fixed-price plan for {{ billing_entity }} ends on {{ end_date }}
 Invoice overdue for {{ billing_entity }} due to stale data
 {% elif notice_reason == "locally_deleted_realm_on_paid_plan" %}
 {{ billing_entity }} on paid plan marked as locally deleted
+{% elif notice_reason == "license_discrepancy" %}
+Check initial licenses invoiced for {{ billing_entity }}
 {% endif %}

templates/zerver/emails/internal_billing_notice.txt

@@ -8,6 +8,11 @@ Recent invoice is overdue for payment.
 Last data upload: {{ last_audit_log_update }}
 {% elif notice_reason == "locally_deleted_realm_on_paid_plan" %}
 Investigate why remote realm is marked as locally deleted when it's on a paid plan.
+{% elif notice_reason == "license_discrepancy" %}
+Discrepancy in licenses when upgraded to current plan.
+
+Licenses paid for: {{ paid_licenses }}.
+Reported licenses in use: {{ current_licenses }}.
 {% endif %}
 
 Support URL: {{ support_url }}