From 2c693f3bd9e87c1a11a7ba422764517b4f940603 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sun, 14 Aug 2022 19:42:55 +0200 Subject: [PATCH] billing: Fix licenses amount check during user signup/invitation. Our seat count calculation is different for guest user than normal users (a number of initial guests are free, and additional marginal guests are worth 1/5 of a seat) - so these checks we apply when a user is being invited or signing up need to know whether it's a guest or non-guest being added. --- corporate/lib/registration.py | 28 ++++++++++++++++++++-------- zerver/actions/invites.py | 9 ++++++++- zerver/forms.py | 4 +++- zerver/tests/test_signup.py | 35 ++++++++++++++++++++++++++++++++++- zerver/views/auth.py | 7 ++++++- zerver/views/registration.py | 9 +++++++-- 6 files changed, 78 insertions(+), 14 deletions(-) diff --git a/corporate/lib/registration.py b/corporate/lib/registration.py index 09d4b7312f..29ce00f5e5 100644 --- a/corporate/lib/registration.py +++ b/corporate/lib/registration.py @@ -3,11 +3,11 @@ from typing import Optional from django.conf import settings from django.utils.translation import gettext as _ -from corporate.lib.stripe import LicenseLimitError, get_latest_seat_count +from corporate.lib.stripe import LicenseLimitError, get_latest_seat_count, get_seat_count from corporate.models import get_current_plan_by_realm from zerver.actions.create_user import send_message_to_signup_notification_stream from zerver.lib.exceptions import InvitationError -from zerver.models import Realm, get_system_bot +from zerver.models import Realm, UserProfile, get_system_bot def generate_licenses_low_warning_message_if_required(realm: Realm) -> Optional[str]: @@ -69,7 +69,7 @@ def send_user_unable_to_signup_message_to_signup_notification_stream( def check_spare_licenses_available_for_adding_new_users( - realm: Realm, number_of_users_to_add: int + realm: Realm, extra_non_guests_count: int = 0, extra_guests_count: int = 0 ) -> None: plan = get_current_plan_by_realm(realm) if ( @@ -79,23 +79,35 @@ def check_spare_licenses_available_for_adding_new_users( ): return - if plan.licenses() < get_latest_seat_count(realm) + number_of_users_to_add: + if plan.licenses() < get_seat_count( + realm, extra_non_guests_count=extra_non_guests_count, extra_guests_count=extra_guests_count + ): raise LicenseLimitError() def check_spare_licenses_available_for_registering_new_user( - realm: Realm, user_email_to_add: str + realm: Realm, + user_email_to_add: str, + role: int, ) -> None: try: - check_spare_licenses_available_for_adding_new_users(realm, 1) + if role == UserProfile.ROLE_GUEST: + check_spare_licenses_available_for_adding_new_users(realm, extra_guests_count=1) + else: + check_spare_licenses_available_for_adding_new_users(realm, extra_non_guests_count=1) except LicenseLimitError: send_user_unable_to_signup_message_to_signup_notification_stream(realm, user_email_to_add) raise -def check_spare_licenses_available_for_inviting_new_users(realm: Realm, num_invites: int) -> None: +def check_spare_licenses_available_for_inviting_new_users( + realm: Realm, extra_non_guests_count: int = 0, extra_guests_count: int = 0 +) -> None: + num_invites = extra_non_guests_count + extra_guests_count try: - check_spare_licenses_available_for_adding_new_users(realm, num_invites) + check_spare_licenses_available_for_adding_new_users( + realm, extra_non_guests_count, extra_guests_count + ) except LicenseLimitError: if num_invites == 1: message = _("All Zulip licenses for this organization are currently in use.") diff --git a/zerver/actions/invites.py b/zerver/actions/invites.py index 1fae864f13..ad5c2b7545 100644 --- a/zerver/actions/invites.py +++ b/zerver/actions/invites.py @@ -138,7 +138,14 @@ def do_invite_users( if settings.BILLING_ENABLED: from corporate.lib.registration import check_spare_licenses_available_for_inviting_new_users - check_spare_licenses_available_for_inviting_new_users(user_profile.realm, num_invites) + if invite_as == PreregistrationUser.INVITE_AS["GUEST_USER"]: + check_spare_licenses_available_for_inviting_new_users( + user_profile.realm, extra_guests_count=num_invites + ) + else: + check_spare_licenses_available_for_inviting_new_users( + user_profile.realm, extra_non_guests_count=num_invites + ) realm = user_profile.realm if not realm.invite_required: diff --git a/zerver/forms.py b/zerver/forms.py index f97d16c47b..6d8e8eff10 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -167,6 +167,7 @@ class HomepageForm(forms.Form): def __init__(self, *args: Any, **kwargs: Any) -> None: self.realm = kwargs.pop("realm", None) self.from_multiuse_invite = kwargs.pop("from_multiuse_invite", False) + self.invited_as = kwargs.pop("invited_as", None) super().__init__(*args, **kwargs) def clean_email(self) -> str: @@ -214,8 +215,9 @@ class HomepageForm(forms.Form): email_is_not_mit_mailing_list(email) if settings.BILLING_ENABLED: + role = self.invited_as if self.invited_as is not None else UserProfile.ROLE_MEMBER try: - check_spare_licenses_available_for_registering_new_user(realm, email) + check_spare_licenses_available_for_registering_new_user(realm, email, role=role) except LicenseLimitError: raise ValidationError( _( diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 25cbf2013b..833ec28a83 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -1299,6 +1299,14 @@ class InviteUserTest(InviteUserBase): result, "All Zulip licenses for this organization are currently in use" ) + with self.settings(BILLING_ENABLED=True): + result = self.invite( + self.nonreg_email("bob"), + ["Denmark"], + invite_as=PreregistrationUser.INVITE_AS["GUEST_USER"], + ) + self.assert_json_success(result) + def test_cross_realm_bot(self) -> None: inviter = self.example_user("hamlet") self.login_user(inviter) @@ -2365,7 +2373,13 @@ so we didn't send them an invitation. We did send invitations to everyone else!" self.assertEqual(response.status_code, 302) self.assertEqual(response["Location"], "http://zulip.testserver/") - self.subscribe_realm_to_monthly_plan_on_manual_license_management(realm, 5, 5) + # We want to simulate the organization having exactly all their licenses + # used, to verify that joining as a regular user is not allowed, + # but as a guest still works (guests are free up to a certain number). + current_seat_count = get_latest_seat_count(realm) + self.subscribe_realm_to_monthly_plan_on_manual_license_management( + realm, current_seat_count, current_seat_count + ) email = self.nonreg_email("bob") prereg_user = PreregistrationUser.objects.create( @@ -2380,6 +2394,25 @@ so we didn't send them an invitation. We did send invitations to everyone else!" ["New members cannot join this organization because all Zulip licenses are"], response ) + guest_prereg_user = PreregistrationUser.objects.create( + email=email, + referred_by=inviter, + realm=realm, + invited_as=PreregistrationUser.INVITE_AS["GUEST_USER"], + ) + confirmation_link = create_confirmation_link( + guest_prereg_user, Confirmation.USER_REGISTRATION + ) + registration_key = confirmation_link.split("/")[-1] + url = "/accounts/register/" + + self.client_post( + url, {"key": registration_key, "from_confirmation": 1, "full_name": "alice"} + ) + response = self.submit_reg_form_for_user(email, "password", key=registration_key) + self.assertEqual(response.status_code, 302) + self.assertEqual(response["Location"], "http://zulip.testserver/") + class InvitationsTestCase(InviteUserBase): def test_do_get_invites_controlled_by_user(self) -> None: diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 9bd30fa2ef..5ef6728ae2 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -211,7 +211,12 @@ def maybe_send_to_registration( else: invited_as = PreregistrationUser.INVITE_AS["MEMBER"] - form = HomepageForm({"email": email}, realm=realm, from_multiuse_invite=from_multiuse_invite) + form = HomepageForm( + {"email": email}, + realm=realm, + from_multiuse_invite=from_multiuse_invite, + invited_as=invited_as, + ) if form.is_valid(): # If the email address is allowed to sign up for an account in # this organization, construct a PreregistrationUser and diff --git a/zerver/views/registration.py b/zerver/views/registration.py index f5d2541e55..96f274d365 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -219,7 +219,7 @@ def accounts_register( if settings.BILLING_ENABLED: try: - check_spare_licenses_available_for_registering_new_user(realm, email) + check_spare_licenses_available_for_registering_new_user(realm, email, role=role) except LicenseLimitError: return render(request, "zerver/no_spare_licenses.html") @@ -697,7 +697,12 @@ def accounts_home( invited_as = multiuse_object.invited_as if request.method == "POST": - form = HomepageForm(request.POST, realm=realm, from_multiuse_invite=from_multiuse_invite) + form = HomepageForm( + request.POST, + realm=realm, + from_multiuse_invite=from_multiuse_invite, + invited_as=invited_as, + ) if form.is_valid(): try: rate_limit_request_by_ip(request, domain="sends_email_by_ip")