billing: Sign and verify the seat count during upgrade.

static/styles/landing-page.scss

@@ -3178,6 +3178,11 @@ nav ul li.active::after {
             -webkit-box-shadow: none;
          }
     }
+
+    #error-message-box {
+        margin-top: 10px;
+        font-weight: 600;
+    }
 }
 
 @keyframes box-shadow-pulse {

templates/zilencer/upgrade.html

@@ -17,10 +17,17 @@
 
         <div class="page-content">
             <div class="main">
+                {% if error_message %}
+                <div class="alert alert-danger" id="error-message-box">
+                    {{ error_message }}
+                </div>
+                {% endif %}
                 <h1>{% trans %}Upgrade to {{ plan }}{% endtrans %}</h1>
                 <form method="post">
                     {{ csrf_input }}
                     <input type="hidden" name="seat_count" value="{{ seat_count }}">
+                    <input type="hidden" name="signed_seat_count" value="{{ signed_seat_count }}">
+                    <input type="hidden" name="salt" value="{{ salt }}">
                     <div class="payment-schedule">
                         <h3>{{ _("Payment schedule") }}</h3>
                         <label>

zilencer/tests/test_stripe.py

@@ -44,6 +44,8 @@ class StripeTest(ZulipTestCase):
         self.stripe_plan_id = 'plan_D7Nh2BtpTvIzYp'
         self.subscription_created = 1529990751
         self.quantity = 8
+
+        self.signed_seat_count, self.salt = sign_string(str(self.quantity))
         Plan.objects.create(nickname=Plan.CLOUD_ANNUAL, stripe_plan_id=self.stripe_plan_id)
 
     @mock.patch("zilencer.lib.stripe.STRIPE_PUBLISHABLE_KEY", "stripe_publishable_key")
@@ -96,7 +98,9 @@ class StripeTest(ZulipTestCase):
         # Click "Make payment" in Stripe Checkout
         response = self.client_post("/upgrade/", {
             'stripeToken': self.token,
-            'seat_count': self.quantity,
+            # TODO: get these values from the response
+            'signed_seat_count': self.signed_seat_count,
+            'salt': self.salt,
             'plan': Plan.CLOUD_ANNUAL})
         # Check that we created a customer and subscription in stripe
         mock_create_customer.assert_called_once_with(
@@ -148,7 +152,8 @@ class StripeTest(ZulipTestCase):
         self.assertEqual('/upgrade/', response.url)
         # Check that non-admins can sign up and pay
         self.client_post("/upgrade/", {'stripeToken': self.token,
-                                       'seat_count': self.quantity,
+                                       'signed_seat_count': self.signed_seat_count,
+                                       'salt': self.salt,
                                        'plan': Plan.CLOUD_ANNUAL})
         # Check that the non-admin hamlet can still access /billing
         response = self.client_get("/billing/")
@@ -173,7 +178,8 @@ class StripeTest(ZulipTestCase):
         # Change the seat count while the user is going through the upgrade flow
         with mock.patch('zilencer.lib.stripe.get_seat_count', return_value=new_seat_count):
             self.client_post("/upgrade/", {'stripeToken': self.token,
-                                           'seat_count': self.quantity,
+                                           'signed_seat_count': self.signed_seat_count,
+                                           'salt': self.salt,
                                            'plan': Plan.CLOUD_ANNUAL})
         # Check that the subscription call used the old quantity, not new_seat_count
         mock_create_subscription.assert_called_once_with(
@@ -200,6 +206,18 @@ class StripeTest(ZulipTestCase):
             event_type=RealmAuditLog.PLAN_QUANTITY_UPDATED).values_list('extra_data', flat=True).first()),
             {'quantity': new_seat_count})
 
+    @mock.patch("zilencer.lib.stripe.STRIPE_PUBLISHABLE_KEY", "stripe_publishable_key")
+    @mock.patch("zilencer.views.STRIPE_PUBLISHABLE_KEY", "stripe_publishable_key")
+    def test_upgrade_with_tampered_seat_count(self) -> None:
+        self.login(self.user.email)
+        result = self.client_post("/upgrade/", {
+            'stripeToken': self.token,
+            'signed_seat_count': "randomsalt",
+            'salt': self.salt,
+            'plan': Plan.CLOUD_ANNUAL
+        })
+        self.assert_in_success_response(["Something went wrong. Please contact"], result)
+
     @mock.patch("zilencer.lib.stripe.STRIPE_PUBLISHABLE_KEY", "stripe_publishable_key")
     @mock.patch("zilencer.views.STRIPE_PUBLISHABLE_KEY", "stripe_publishable_key")
     @mock.patch("stripe.Customer.retrieve", side_effect=mock_retrieve_customer)

zilencer/views.py

@@ -1,7 +1,9 @@
 from typing import Any, Dict, Optional, Union, cast
+import logging
 
 from django.core.exceptions import ValidationError
 from django.core.validators import validate_email, URLValidator
+from django.core import signing
 from django.db import IntegrityError
 from django.http import HttpRequest, HttpResponse, HttpResponseRedirect
 from django.utils import timezone
@@ -26,10 +28,12 @@ from zerver.views.push_notifications import validate_token
 from zilencer.lib.stripe import STRIPE_PUBLISHABLE_KEY, StripeError, \
     do_create_customer_with_payment_source, do_subscribe_customer_to_plan, \
     get_stripe_customer, get_upcoming_invoice, payment_source, \
-    get_seat_count, extract_current_subscription
+    get_seat_count, extract_current_subscription, sign_string, unsign_string
 from zilencer.models import RemotePushDeviceToken, RemoteZulipServer, \
     Customer, Plan
 
+billing_logger = logging.getLogger('zilencer.stripe')
+
 def validate_entity(entity: Union[UserProfile, RemoteZulipServer]) -> None:
     if not isinstance(entity, RemoteZulipServer):
         raise JsonableError(err_("Must validate with valid Zulip server API key"))
@@ -158,32 +162,43 @@ def remote_server_notify_push(request: HttpRequest, entity: Union[UserProfile, R
 @zulip_login_required
 def initial_upgrade(request: HttpRequest) -> HttpResponse:
     user = request.user
+    error_message = ""
+
     if Customer.objects.filter(realm=user.realm).exists():
         return HttpResponseRedirect(reverse('zilencer.views.billing_home'))
 
     if request.method == 'POST':
-        stripe_customer = do_create_customer_with_payment_source(user, request.POST['stripeToken'])
+        try:
-        # TODO: the current way this is done is subject to tampering by the user.
+            seat_count = int(unsign_string(request.POST['signed_seat_count'], request.POST['salt']))
-        seat_count = int(request.POST['seat_count'])
+        except signing.BadSignature:
-        if seat_count < 1:
+            billing_logger.warning("Tampered seat count during realm upgrade. user: %s, realm: %s (%s)."
-            raise AssertionError('seat_count is less than 1')
+                                   % (user.id, user.realm.id, user.realm.string_id))
-        do_subscribe_customer_to_plan(
+            error_message = "Something went wrong. Please contact support@zulipchat.com"
-            stripe_customer=stripe_customer,
-            stripe_plan_id=Plan.objects.get(nickname=request.POST['plan']).stripe_plan_id,
-            seat_count=seat_count,
-            # TODO: billing address details are passed to us in the request;
-            # use that to calculate taxes.
-            tax_percent=0)
-        # TODO: check for errors and raise/send to frontend
-        return HttpResponseRedirect(reverse('zilencer.views.billing_home'))
 
+        if not error_message:
+            stripe_customer = do_create_customer_with_payment_source(user, request.POST['stripeToken'])
+            do_subscribe_customer_to_plan(
+                stripe_customer=stripe_customer,
+                stripe_plan_id=Plan.objects.get(nickname=request.POST['plan']).stripe_plan_id,
+                seat_count=seat_count,
+                # TODO: billing address details are passed to us in the request;
+                # use that to calculate taxes.
+                tax_percent=0)
+            # TODO: check for errors and raise/send to frontend
+            return HttpResponseRedirect(reverse('zilencer.views.billing_home'))
+
+    seat_count = get_seat_count(user.realm)
+    signed_seat_count, salt = sign_string(str(seat_count))
     context = {
         'publishable_key': STRIPE_PUBLISHABLE_KEY,
         'email': user.email,
-        'seat_count': get_seat_count(user.realm),
+        'seat_count': seat_count,
+        'signed_seat_count': signed_seat_count,
+        'salt': salt,
         'plan': "Zulip Premium",
         'nickname_monthly': Plan.CLOUD_MONTHLY,
         'nickname_annual': Plan.CLOUD_ANNUAL,
+        'error_message': error_message,
     }  # type: Dict[str, Any]
     return render(request, 'zilencer/upgrade.html', context=context)