billing: Restrict access to billing page to realm owners and billing admins.

This commit is contained in:
Vishnu KS 2020-07-14 12:40:39 +00:00 committed by Tim Abbott
parent a6519e7b8f
commit cb01a7f599
6 changed files with 72 additions and 53 deletions

View File

@ -257,7 +257,7 @@ class StripeTestCase(ZulipTestCase):
self.example_email('hamlet'),
self.example_email('iago'),
self.example_email('othello'),
self.example_email('prospero'),
self.example_email('desdemona'),
self.example_email('polonius'), # guest
self.example_email('default_bot'), # bot
]
@ -855,12 +855,8 @@ class StripeTest(StripeTestCase):
@mock_stripe()
def test_billing_page_permissions(self, *mocks: Mock) -> None:
hamlet = self.example_user('hamlet')
iago = self.example_user('iago')
cordelia = self.example_user('cordelia')
# Check that non-admins can access /upgrade via /billing, when there is no Customer object
self.login_user(hamlet)
self.login_user(self.example_user('hamlet'))
response = self.client_get("/billing/")
self.assertEqual(response.status_code, 302)
self.assertEqual('/upgrade/', response.url)
@ -868,15 +864,20 @@ class StripeTest(StripeTestCase):
self.upgrade()
# Check that the non-admin hamlet can still access /billing
response = self.client_get("/billing/")
self.assert_in_success_response(["for billing history or to make changes"], response)
# Check admins can access billing, even though they are not a billing admin
self.login_user(iago)
self.assert_in_success_response(["Your current plan is"], response)
# Check realm owners can access billing, even though they are not a billing admin
desdemona = self.example_user('desdemona')
desdemona.role = UserProfile.ROLE_REALM_OWNER
desdemona.save(update_fields=["role"])
self.login_user(self.example_user('desdemona'))
response = self.client_get("/billing/")
self.assert_in_success_response(["for billing history or to make changes"], response)
# Check that a non-admin, non-billing admin user does not have access
self.login_user(cordelia)
self.assert_in_success_response(["Your current plan is"], response)
# Check that member who is not a billing admin does not have access
self.login_user(self.example_user('cordelia'))
response = self.client_get("/billing/")
self.assert_in_success_response(["You must be an organization administrator"], response)
self.assert_in_success_response(["You must be an organization owner or a billing administrator"], response)
@mock_stripe(tested_timestamp_fields=["created"])
def test_upgrade_by_card_with_outdated_seat_count(self, *mocks: Mock) -> None:
@ -1109,7 +1110,7 @@ class StripeTest(StripeTestCase):
self.login_user(self.example_user("othello"))
response = self.client_get("/billing/")
self.assert_in_success_response(["You must be an organization administrator or a billing administrator to view this page."], response)
self.assert_in_success_response(["You must be an organization owner or a billing administrator to view this page."], response)
def test_redirect_for_billing_home(self) -> None:
user = self.example_user("iago")
@ -1824,21 +1825,45 @@ class RequiresBillingAccessTest(ZulipTestCase):
hamlet.is_billing_admin = True
hamlet.save(update_fields=["is_billing_admin"])
def verify_non_admins_blocked_from_endpoint(
self, url: str, request_data: Dict[str, Any]={}) -> None:
cordelia = self.example_user('cordelia')
self.login_user(cordelia)
response = self.client_post(url, request_data)
self.assert_json_error_contains(response, "Must be a billing administrator or an organization")
desdemona = self.example_user('desdemona')
desdemona.role = UserProfile.ROLE_REALM_OWNER
desdemona.save(update_fields=["role"])
def test_who_can_access_json_endpoints(self) -> None:
# Billing admins have access
self.login_user(self.example_user('hamlet'))
with patch("corporate.views.do_replace_payment_source") as mocked1:
response = self.client_post("/json/billing/sources/change",
{'stripe_token': ujson.dumps('token')})
self.assert_json_success(response)
mocked1.assert_called()
# Realm owners have access, even if they are not billing admins
self.login_user(self.example_user('desdemona'))
with patch("corporate.views.do_replace_payment_source") as mocked2:
response = self.client_post("/json/billing/sources/change",
{'stripe_token': ujson.dumps('token')})
self.assert_json_success(response)
mocked2.assert_called()
def test_who_cant_access_json_endpoints(self) -> None:
def verify_user_cant_access_endpoint(user: UserProfile, url: str, request_data: Dict[str, Any]={}) -> None:
self.login_user(user)
response = self.client_post(url, request_data)
self.assert_json_error_contains(response, "Must be a billing administrator or an organization owner")
def test_non_admins_blocked_from_json_endpoints(self) -> None:
params: List[Tuple[str, Dict[str, Any]]] = [
("/json/billing/sources/change", {'stripe_token': ujson.dumps('token')}),
("/json/billing/plan/change", {'status': ujson.dumps(1)}),
]
for (url, data) in params:
self.verify_non_admins_blocked_from_endpoint(url, data)
# Guests can't access
verify_user_cant_access_endpoint(self.example_user("polonius"), url, data)
# Members (not billing admin) can't access
verify_user_cant_access_endpoint(self.example_user("cordelia"), url, data)
# Realm admins (not billing admin) can't access
verify_user_cant_access_endpoint(self.example_user("iago"), url, data)
# Make sure that we are testing all the JSON endpoints
# Quite a hack, but probably fine for now
@ -1851,25 +1876,6 @@ class RequiresBillingAccessTest(ZulipTestCase):
self.assertEqual(len(json_endpoints), len(params))
def test_admins_and_billing_admins_can_access(self) -> None:
# Billing admins have access
hamlet = self.example_user('hamlet')
iago = self.example_user('iago')
self.login_user(hamlet)
with patch("corporate.views.do_replace_payment_source") as mocked1:
response = self.client_post("/json/billing/sources/change",
{'stripe_token': ujson.dumps('token')})
self.assert_json_success(response)
mocked1.assert_called()
# Realm admins have access, even if they are not billing admins
self.login_user(iago)
with patch("corporate.views.do_replace_payment_source") as mocked2:
response = self.client_post("/json/billing/sources/change",
{'stripe_token': ujson.dumps('token')})
self.assert_json_success(response)
mocked2.assert_called()
class BillingHelpersTest(ZulipTestCase):
def test_next_month(self) -> None:
anchor = datetime(2019, 12, 31, 1, 2, 3, tzinfo=timezone.utc)

View File

@ -171,7 +171,7 @@
</div>
{% else %}
<p>
You must be an organization administrator or a billing administrator to view this page.
You must be an organization owner or a billing administrator to view this page.
</p>
{% endif %}
</div>

View File

@ -6,11 +6,17 @@ There are several possible roles in a Zulip organization.
organization settings, and billing.
* **Organization Administrator**: Can manage users, public streams,
organization settings, and billing. Cannot create or demote
and organization settings. Can upgrade the organization to a paid
plan and become a billing admin. Cannot create or demote
organization owners.
* **Member**: Has access to all public streams. This is the default role for
most users.
* **Member**: Has access to all public streams. Can upgrade
the organization to a paid plan and become a billing administrator.
Member is the default role for most users.
* **Billing Administrator**: Member or organization administrator who
upgrades the organization to a paid plan becomes a billing administrator.
Can manage billing in addition to the existing privileges.
* **Guest**: Can only access streams they've been added to. Cannot create
new streams or invite other users.

View File

@ -133,12 +133,11 @@ def require_realm_admin(func: ViewFuncT) -> ViewFuncT:
def require_billing_access(func: ViewFuncT) -> ViewFuncT:
@wraps(func)
def wrapper(request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object) -> HttpResponse:
if not user_profile.is_realm_admin and not user_profile.is_billing_admin:
raise JsonableError(_("Must be a billing administrator or an organization administrator"))
if not user_profile.has_billing_access:
raise JsonableError(_("Must be a billing administrator or an organization owner"))
return func(request, user_profile, *args, **kwargs)
return cast(ViewFuncT, wrapper) # https://github.com/python/mypy/issues/1927
def get_client_name(request: HttpRequest) -> str:
# If the API request specified a client in the request content,
# that has priority. Otherwise, extract the client from the

View File

@ -1199,7 +1199,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
@property
def has_billing_access(self) -> bool:
return self.is_realm_admin or self.is_billing_admin
return self.is_realm_owner or self.is_billing_admin
@property
def is_realm_owner(self) -> bool:

View File

@ -317,7 +317,7 @@ class HomeTest(ZulipTestCase):
result = self._get_home_page()
self.assertEqual(result.status_code, 200)
self.assert_length(cache_mock.call_args_list, 6)
self.assert_length(queries, 40)
self.assert_length(queries, 39)
def test_num_queries_with_streams(self) -> None:
main_user = self.example_user('hamlet')
@ -680,20 +680,28 @@ class HomeTest(ZulipTestCase):
def test_show_billing(self) -> None:
customer = Customer.objects.create(realm=get_realm("zulip"), stripe_customer_id="cus_id")
user = self.example_user('desdemona')
# realm admin, but no CustomerPlan -> no billing link
user = self.example_user('iago')
# realm owner, but no CustomerPlan -> no billing link
user.role = UserProfile.ROLE_REALM_OWNER
user.save(update_fields=["role"])
self.login_user(user)
result_html = self._get_home_page().content.decode('utf-8')
self.assertNotIn('Billing', result_html)
# realm admin, with inactive CustomerPlan -> show billing link
# realm owner, with inactive CustomerPlan -> show billing link
CustomerPlan.objects.create(customer=customer, billing_cycle_anchor=timezone_now(),
billing_schedule=CustomerPlan.ANNUAL, next_invoice_date=timezone_now(),
tier=CustomerPlan.STANDARD, status=CustomerPlan.ENDED)
result_html = self._get_home_page().content.decode('utf-8')
self.assertIn('Billing', result_html)
# realm admin, with CustomerPlan -> no billing link
user.role = UserProfile.ROLE_REALM_ADMINISTRATOR
user.save(update_fields=["role"])
result_html = self._get_home_page().content.decode('utf-8')
self.assertNotIn('Billing', result_html)
# billing admin, with CustomerPlan -> show billing link
user.role = UserProfile.ROLE_MEMBER
user.is_billing_admin = True