From e39f400f94c950eec5cebc804e366fdcca30f428 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Wed, 28 Feb 2024 20:03:34 +0100 Subject: [PATCH] remote_billing: Make "plan management" always available. Just shows a config error page if the bouncer is not enabled. Uses a new endpoint for this so that it can work nicely for both browser and desktop app clients. It's necessary, because the desktop app expects to get a json response with either an error or billing_access_url to redirect to. Showing a nice config error page can't be done via the json error mechanism, so instead we just serve a redirect to the new error page, which the app will open in the browser in a new window or tab. --- corporate/tests/test_remote_billing.py | 59 +++++++++++++++++-- ...remote_billing_bouncer_not_configured.html | 9 +++ zerver/lib/home.py | 8 ++- zerver/lib/test_helpers.py | 3 + zerver/tests/test_home.py | 32 ++++++---- zerver/views/push_notifications.py | 17 +++++- zproject/urls.py | 5 ++ 7 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 templates/zerver/config_error/remote_billing_bouncer_not_configured.html diff --git a/corporate/tests/test_remote_billing.py b/corporate/tests/test_remote_billing.py index 6fcf5b58ec..76aeff1bdc 100644 --- a/corporate/tests/test_remote_billing.py +++ b/corporate/tests/test_remote_billing.py @@ -199,12 +199,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", @@ -240,6 +269,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 a3f0f29e64..028d0fe3e4 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 ea53031ce4..bcfce21668 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 a165b00c46..9bb7e2c39d 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -973,7 +973,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") @@ -990,7 +990,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) @@ -999,7 +999,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): @@ -1029,6 +1029,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 @@ -1039,6 +1045,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 @@ -1049,7 +1060,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) @@ -1083,7 +1094,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 @@ -1093,7 +1104,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() @@ -1102,13 +1113,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 864a5352d0..c9f61245e1 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -99,6 +99,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,