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

@@ -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)
+            with self.settings(CORPORATE_ENABLED=True):
-            self.assertEqual(result.status_code, 404)
+                result = self.client_get(self_hosted_billing_url)
-            self.assert_in_response("Page not found (404)", result)
+                self.assertEqual(result.status_code, 404)
+                self.assert_in_response("Page not found (404)", result)
 
-            result = self.client_get(self_hosted_billing_json_url)
+            with self.settings(CORPORATE_ENABLED=False):
-            self.assert_json_error(result, "Server doesn't use the push notification service", 404)
+                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")

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
+        <a href="{{ doc_url }}">documentation</a>.
+    {% endtrans %}
+{% endblock %}

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

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),
         }
 

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.
+        # remote billing should be shown anyway, as the billing endpoint
-        with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=None):
+        # 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")

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")

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,