diff --git a/corporate/tests/test_remote_billing.py b/corporate/tests/test_remote_billing.py index 4180cfb18f..cc3e05c8ec 100644 --- a/corporate/tests/test_remote_billing.py +++ b/corporate/tests/test_remote_billing.py @@ -182,12 +182,41 @@ class SelfHostedBillingEndpointBasicTest(RemoteRealmBillingTestCase): self_hosted_billing_json_url = "/json/self-hosted-billing" with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=None): - result = self.client_get(self_hosted_billing_url) - self.assertEqual(result.status_code, 404) - self.assert_in_response("Page not found (404)", result) + with self.settings(CORPORATE_ENABLED=True): + result = self.client_get(self_hosted_billing_url) + self.assertEqual(result.status_code, 404) + self.assert_in_response("Page not found (404)", result) - result = self.client_get(self_hosted_billing_json_url) - self.assert_json_error(result, "Server doesn't use the push notification service", 404) + with self.settings(CORPORATE_ENABLED=False): + result = self.client_get(self_hosted_billing_url) + self.assertEqual(result.status_code, 302) + redirect_url = result["Location"] + self.assertEqual(redirect_url, "/self-hosted-billing/not-configured/") + + with self.assertLogs("django.request"): + result = self.client_get(redirect_url) + self.assert_in_response( + "This server is not configured to use push notifications.", result + ) + + with self.settings(CORPORATE_ENABLED=True): + result = self.client_get(self_hosted_billing_json_url) + self.assert_json_error( + result, "Server doesn't use the push notification service", 404 + ) + + with self.settings(CORPORATE_ENABLED=False): + result = self.client_get(self_hosted_billing_json_url) + self.assert_json_success(result) + + redirect_url = result.json()["billing_access_url"] + self.assertEqual(redirect_url, "/self-hosted-billing/not-configured/") + + with self.assertLogs("django.request"): + result = self.client_get(redirect_url) + self.assert_in_response( + "This server is not configured to use push notifications.", result + ) with mock.patch( "zerver.views.push_notifications.send_to_push_bouncer", @@ -223,6 +252,26 @@ class SelfHostedBillingEndpointBasicTest(RemoteRealmBillingTestCase): @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com") class RemoteBillingAuthenticationTest(RemoteRealmBillingTestCase): + def test_self_hosted_config_error_page(self) -> None: + with self.settings( + CORPORATE_ENABLED=False, PUSH_NOTIFICATION_BOUNCER_URL=None + ), self.assertLogs("django.request"): + result = self.client_get("/self-hosted-billing/not-configured/") + self.assertEqual(result.status_code, 500) + self.assert_in_response( + "This server is not configured to use push notifications.", result + ) + + # The page doesn't make sense if PUSH_NOTIFICATION_BOUNCER_URL is configured. + with self.settings(CORPORATE_ENABLED=False): + result = self.client_get("/self-hosted-billing/not-configured/") + self.assertEqual(result.status_code, 404) + + # Also doesn't make sense on zulipchat.com (where CORPORATE_ENABLED is True). + with self.settings(CORPORATE_ENABLED=True, PUSH_NOTIFICATION_BOUNCER_URL=None): + result = self.client_get("/self-hosted-billing/not-configured/") + self.assertEqual(result.status_code, 404) + @responses.activate def test_remote_billing_authentication_flow(self) -> None: self.login("desdemona") diff --git a/templates/zerver/config_error/remote_billing_bouncer_not_configured.html b/templates/zerver/config_error/remote_billing_bouncer_not_configured.html new file mode 100644 index 0000000000..8538be4780 --- /dev/null +++ b/templates/zerver/config_error/remote_billing_bouncer_not_configured.html @@ -0,0 +1,9 @@ +{% extends "zerver/config_error/container.html" %} + +{% block error_content %} + {% trans doc_url="https://zulip.readthedocs.io/en/latest/production/mobile-push-notifications.html" %} + This server is not configured to use push notifications. For instructions on how to + configure push notifications, please see the + documentation. + {% endtrans %} +{% endblock %} diff --git a/zerver/lib/home.py b/zerver/lib/home.py index 642bb9b89c..a57adc3ce2 100644 --- a/zerver/lib/home.py +++ b/zerver/lib/home.py @@ -16,7 +16,6 @@ from zerver.lib.i18n import ( get_language_translation_data, ) from zerver.lib.narrow_helpers import NarrowTerm -from zerver.lib.push_notifications import uses_notification_bouncer from zerver.lib.realm_description import get_realm_rendered_description from zerver.lib.request import RequestNotes from zerver.models import Message, Realm, Stream, UserProfile @@ -82,8 +81,13 @@ def get_billing_info(user_profile: Optional[UserProfile]) -> BillingInfo: show_billing = False show_plans = False sponsorship_pending = False + + # We want to always show the remote billing option as long as the user is authorized, + # except on zulipchat.com where it's not applicable. show_remote_billing = ( - user_profile is not None and user_profile.has_billing_access and uses_notification_bouncer() + (not settings.CORPORATE_ENABLED) + and user_profile is not None + and user_profile.has_billing_access ) # This query runs on home page load, so we want to avoid diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 65f8553238..01b28f2c02 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -528,6 +528,9 @@ def write_instrumentation_reports(full_suite: bool, include_webhooks: bool) -> N # TODO: This endpoint and the rest of its system are a work in progress, # we are not testing it yet. "self-hosted-billing/", + # This endpoint only returns 500 and 404 codes, so it doesn't get picked up + # by find_pattern above and therefore needs to be exempt. + "self-hosted-billing/not-configured/", *(webhook.url for webhook in WEBHOOK_INTEGRATIONS if not include_webhooks), } diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index ab997bc9c7..78af3baf72 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -837,7 +837,7 @@ class HomeTest(ZulipTestCase): self.assertFalse(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # realm owner, with inactive CustomerPlan and realm plan_type SELF_HOSTED -> show only billing link customer = Customer.objects.create(realm=get_realm("zulip"), stripe_customer_id="cus_id") @@ -854,7 +854,7 @@ class HomeTest(ZulipTestCase): self.assertTrue(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # realm owner, with inactive CustomerPlan and realm plan_type LIMITED -> show billing link and plans do_change_realm_plan_type(user.realm, Realm.PLAN_TYPE_LIMITED, acting_user=None) @@ -863,7 +863,7 @@ class HomeTest(ZulipTestCase): self.assertTrue(billing_info.show_billing) self.assertTrue(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # Always false without CORPORATE_ENABLED with self.settings(CORPORATE_ENABLED=False): @@ -893,6 +893,12 @@ class HomeTest(ZulipTestCase): self.assertFalse(billing_info.sponsorship_pending) self.assertFalse(billing_info.show_remote_billing) + # Self-hosted servers show remote billing, but not for a user without + # billing access permission. + with self.settings(CORPORATE_ENABLED=False): + billing_info = get_billing_info(user) + self.assertFalse(billing_info.show_remote_billing) + # billing admin, with CustomerPlan and realm plan_type STANDARD -> show only billing link user.role = UserProfile.ROLE_MEMBER user.is_billing_admin = True @@ -903,6 +909,11 @@ class HomeTest(ZulipTestCase): self.assertTrue(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) + self.assertFalse(billing_info.show_remote_billing) + + # Self-hosted servers show remote billing for billing admins. + with self.settings(CORPORATE_ENABLED=False): + billing_info = get_billing_info(user) self.assertTrue(billing_info.show_remote_billing) # billing admin, with CustomerPlan and realm plan_type PLUS -> show only billing link @@ -913,7 +924,7 @@ class HomeTest(ZulipTestCase): self.assertTrue(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # member, with CustomerPlan and realm plan_type STANDARD -> neither billing link or plans do_change_realm_plan_type(user.realm, Realm.PLAN_TYPE_STANDARD, acting_user=None) @@ -947,7 +958,7 @@ class HomeTest(ZulipTestCase): self.assertFalse(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # billing admin, with sponsorship pending and realm plan_type SELF_HOSTED -> show only sponsorship pending link customer.sponsorship_pending = True @@ -957,7 +968,7 @@ class HomeTest(ZulipTestCase): self.assertFalse(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertTrue(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # billing admin, no customer object and realm plan_type SELF_HOSTED -> no links customer.delete() @@ -966,13 +977,14 @@ class HomeTest(ZulipTestCase): self.assertFalse(billing_info.show_billing) self.assertFalse(billing_info.show_plans) self.assertFalse(billing_info.sponsorship_pending) - self.assertTrue(billing_info.show_remote_billing) + self.assertFalse(billing_info.show_remote_billing) # If the server doesn't have the push bouncer configured, - # don't show remote billing. - with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=None): + # remote billing should be shown anyway, as the billing endpoint + # is supposed show a useful error page. + with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=None, CORPORATE_ENABLED=False): billing_info = get_billing_info(user) - self.assertFalse(billing_info.show_remote_billing) + self.assertTrue(billing_info.show_remote_billing) def test_promote_sponsoring_zulip_in_realm(self) -> None: realm = get_realm("zulip") diff --git a/zerver/views/push_notifications.py b/zerver/views/push_notifications.py index 5283a0ed74..fdc03d54a9 100644 --- a/zerver/views/push_notifications.py +++ b/zerver/views/push_notifications.py @@ -4,6 +4,7 @@ from typing import Optional from django.conf import settings from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import render +from django.urls import reverse from django.utils.translation import gettext as _ from zerver.decorator import human_users_only, zulip_login_required @@ -33,6 +34,7 @@ from zerver.lib.response import json_success from zerver.lib.typed_endpoint import typed_endpoint from zerver.lib.validator import check_string from zerver.models import PushDeviceToken, UserProfile +from zerver.views.errors import config_error def check_app_id(var_name: str, val: object) -> str: @@ -123,7 +125,11 @@ def self_hosting_auth_view_common( request: HttpRequest, user_profile: UserProfile, next_page: Optional[str] = None ) -> str: if not uses_notification_bouncer(): - raise ResourceNotFoundError(_("Server doesn't use the push notification service")) + if settings.CORPORATE_ENABLED: + # This endpoint makes no sense on zulipchat.com, so just 404. + raise ResourceNotFoundError(_("Server doesn't use the push notification service")) + else: + return reverse(self_hosting_auth_not_configured) if not user_profile.has_billing_access: # nocoverage # We may want to replace this with an html error page at some point, @@ -210,3 +216,12 @@ def self_hosting_auth_json_endpoint( redirect_url = self_hosting_auth_view_common(request, user_profile, next_page) return json_success(request, data={"billing_access_url": redirect_url}) + + +def self_hosting_auth_not_configured(request: HttpRequest) -> HttpResponse: + if settings.CORPORATE_ENABLED or uses_notification_bouncer(): + # This error page should only be available if the config error + # is actually real. + return render(request, "404.html", status=404) + + return config_error(request, "remote_billing_bouncer_not_configured") diff --git a/zproject/urls.py b/zproject/urls.py index dd23ed8e00..f0200dcaf6 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -93,6 +93,7 @@ from zerver.views.push_notifications import ( remove_android_reg_id, remove_apns_device_token, self_hosting_auth_json_endpoint, + self_hosting_auth_not_configured, self_hosting_auth_redirect_endpoint, send_test_push_notification_api, ) @@ -828,6 +829,10 @@ urls += [ self_hosting_auth_redirect_endpoint, name="self_hosting_auth_redirect_endpoint", ), + path( + "self-hosted-billing/not-configured/", + self_hosting_auth_not_configured, + ), rest_path( "json/self-hosted-billing", GET=self_hosting_auth_json_endpoint,