remote_billing: Fix /self-hosted-billing/ handling for desktop app.

When you click "Plan management", the desktop app opens
/self-hosted-billing/ in your browser immediately. So that works badly
if you're already logged into another account in the browser, since that
session will be used and it may be for a different user account than in
the desktop app, causing unintended behavior.

The solution is to replace the on click behavior for "Plan management"
in the desktop app case, to instead make a request to a new endpoint
/json/self-hosted-billing, which provides the billing access url in a
json response. The desktop app takes that URL and window.open()s it (in
the browser). And so a remote billing session for the intended user will
be obtained.

corporate/tests/test_remote_billing.py

@@ -24,6 +24,7 @@ from corporate.models import (
 )
 from corporate.views.remote_billing_page import generate_confirmation_link_for_server_deactivation
 from zerver.actions.realm_settings import do_deactivate_realm
+from zerver.lib.exceptions import RemoteRealmServerMismatchError
 from zerver.lib.remote_server import send_server_data_to_push_bouncer
 from zerver.lib.test_classes import BouncerTestCase
 from zerver.lib.timestamp import datetime_to_timestamp
@@ -175,6 +176,57 @@ class RemoteRealmBillingTestCase(BouncerTestCase):
         return result
 
 
+@override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com")
+class SelfHostedBillingEndpointBasicTest(RemoteRealmBillingTestCase):
+    @responses.activate
+    def test_self_hosted_billing_endpoints(self) -> None:
+        self.login("desdemona")
+
+        self.add_mock_response()
+
+        self_hosted_billing_url = "/self-hosted-billing/"
+        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)
+
+            result = self.client_get(self_hosted_billing_json_url)
+            self.assert_json_error(result, "Server doesn't use the push notification service", 404)
+
+        with mock.patch(
+            "zerver.views.push_notifications.send_to_push_bouncer",
+            side_effect=RemoteRealmServerMismatchError,
+        ):
+            result = self.client_get(self_hosted_billing_url)
+            self.assertEqual(result.status_code, 403)
+            self.assert_in_response("Unexpected Zulip server registration", result)
+
+            result = self.client_get(self_hosted_billing_json_url)
+            self.assert_json_error(
+                result,
+                "Your organization is registered to a different Zulip server. Please contact Zulip support for assistance in resolving this issue.",
+                403,
+            )
+
+        # Now test successes. We only check that an url for accessing the remote billing system
+        # is returned (in the appropriate format - redirect or json data, depending on the endpoint).
+        # We don't need to test that returned URL beyond that, because that's just the full auth flow,
+        # which gets tested properly in other tests.
+        result = self.client_get(self_hosted_billing_url)
+        self.assertEqual(result.status_code, 302)
+        self.assertIn("http://selfhosting.testserver/remote-billing-login/", result["Location"])
+
+        result = self.client_get(self_hosted_billing_json_url)
+        self.assert_json_success(result)
+        data = result.json()
+        self.assertEqual(sorted(data.keys()), ["billing_access_url", "msg", "result"])
+        self.assertIn(
+            "http://selfhosting.testserver/remote-billing-login/", data["billing_access_url"]
+        )
+
+
 @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com")
 class RemoteBillingAuthenticationTest(RemoteRealmBillingTestCase):
     @responses.activate

web/src/desktop_integration.js

@@ -2,6 +2,8 @@ import $ from "jquery";
 
 import * as browser_history from "./browser_history";
 import * as channel from "./channel";
+import * as feedback_widget from "./feedback_widget";
+import {$t} from "./i18n";
 import * as message_store from "./message_store";
 import * as narrow from "./narrow";
 import * as stream_data from "./stream_data";
@@ -63,3 +65,32 @@ if (window.electron_bridge !== undefined) {
         });
     });
 }
+
+export function initialize() {
+    if (window.electron_bridge === undefined) {
+        return;
+    }
+
+    $(document).on("click", "#open-self-hosted-billing", (event) => {
+        event.preventDefault();
+
+        const url = "/json/self-hosted-billing";
+
+        channel.get({
+            url,
+            success(data) {
+                window.open(data.billing_access_url, "_blank", "noopener,noreferrer");
+            },
+            error(xhr) {
+                if (xhr.responseJSON?.msg) {
+                    feedback_widget.show({
+                        populate($container) {
+                            $container.text(xhr.responseJSON.msg);
+                        },
+                        title_text: $t({defaultMessage: "Error"}),
+                    });
+                }
+            },
+        });
+    });
+}

web/src/ui_init.js

@@ -34,6 +34,7 @@ import * as composebox_typeahead from "./composebox_typeahead";
 import * as condense from "./condense";
 import * as copy_and_paste from "./copy_and_paste";
 import * as dark_theme from "./dark_theme";
+import * as desktop_integration from "./desktop_integration";
 import * as desktop_notifications from "./desktop_notifications";
 import * as drafts from "./drafts";
 import * as drafts_overlay_ui from "./drafts_overlay_ui";
@@ -701,6 +702,7 @@ export function initialize_everything() {
     fenced_code.initialize(generated_pygments_data);
     message_edit_history.initialize();
     hotkey.initialize();
+    desktop_integration.initialize();
 
     $("#app-loading").addClass("loaded");
 }

web/templates/gear_menu_popover.hbs

@@ -135,7 +135,7 @@
                 {{/if}}
                 {{#if show_remote_billing }}
                 <li class="link-item navbar-dropdown-menu-inner-list-item">
-                    <a href="/self-hosted-billing/" target="_blank" rel="noopener noreferrer" class="navigate-link-on-enter navbar-dropdown-menu-link">
+                    <a href="/self-hosted-billing/" id="open-self-hosted-billing" target="_blank" rel="noopener noreferrer" class="navigate-link-on-enter navbar-dropdown-menu-link">
                         <i class="navbar-dropdown-icon zulip-icon zulip-icon-rocket" aria-hidden="true"></i> {{t 'Plan management' }}
                     </a>
                 </li>

zerver/views/push_notifications.py

@@ -12,6 +12,7 @@ from zerver.lib.exceptions import (
     MissingRemoteRealmError,
     OrganizationOwnerRequiredError,
     RemoteRealmServerMismatchError,
+    ResourceNotFoundError,
 )
 from zerver.lib.push_notifications import (
     InvalidPushDeviceTokenError,
@@ -118,32 +119,25 @@ def send_test_push_notification_api(
     return json_success(request)
 
 
-@zulip_login_required
+def self_hosting_auth_view_common(
-@typed_endpoint
+    request: HttpRequest, user_profile: UserProfile, next_page: Optional[str] = None
-def self_hosting_auth_redirect(
+) -> str:
-    request: HttpRequest,
+    if not uses_notification_bouncer():
-    *,
+        raise ResourceNotFoundError(_("Server doesn't use the push notification service"))
-    next_page: Optional[str] = None,
-) -> HttpResponse:
-    if not uses_notification_bouncer():  # nocoverage
-        return render(request, "404.html", status=404)
 
-    user = request.user
+    if not user_profile.has_billing_access:  # nocoverage
-    assert user.is_authenticated
-    assert isinstance(user, UserProfile)
-    if not user.has_billing_access:  # nocoverage
         # We may want to replace this with an html error page at some point,
         # but this endpoint shouldn't be accessible via the UI to an unauthorized
-        # user - and they need to directly enter the URL in their browser. So a json
+        # user_profile - and they need to directly enter the URL in their browser. So a json
         # error may be sufficient.
         raise OrganizationOwnerRequiredError
 
-    realm_info = get_realms_info_for_push_bouncer(user.realm_id)[0]
+    realm_info = get_realms_info_for_push_bouncer(user_profile.realm_id)[0]
 
     user_info = UserDataForRemoteBilling(
-        uuid=user.uuid,
+        uuid=user_profile.uuid,
-        email=user.delivery_email,
+        email=user_profile.delivery_email,
-        full_name=user.full_name,
+        full_name=user_profile.full_name,
     )
 
     post_data = {
@@ -164,12 +158,55 @@ def self_hosting_auth_redirect(
         # Upload realm info and re-try. It should work now.
         send_server_data_to_push_bouncer(consider_usage_statistics=False)
         result = send_to_push_bouncer("POST", "server/billing", post_data)
-    except RemoteRealmServerMismatchError:  # nocoverage
-        return render(request, "zilencer/remote_realm_server_mismatch_error.html", status=403)
 
     if result["result"] != "success":  # nocoverage
         raise JsonableError(_("Error returned by the bouncer: {result}").format(result=result))
 
     redirect_url = result["billing_access_url"]
     assert isinstance(redirect_url, str)
+    return redirect_url
+
+
+@zulip_login_required
+@typed_endpoint
+def self_hosting_auth_redirect_endpoint(
+    request: HttpRequest,
+    *,
+    next_page: Optional[str] = None,
+) -> HttpResponse:
+    """
+    This endpoint is used by the web app running in the browser. We serve HTML
+    error pages, and in case of success a simple redirect to the remote billing
+    access link received from the bouncer.
+    """
+
+    user = request.user
+    assert user.is_authenticated
+    assert isinstance(user, UserProfile)
+
+    try:
+        redirect_url = self_hosting_auth_view_common(request, user, next_page)
+    except ResourceNotFoundError:
+        return render(request, "404.html", status=404)
+    except RemoteRealmServerMismatchError:
+        return render(request, "zilencer/remote_realm_server_mismatch_error.html", status=403)
+
     return HttpResponseRedirect(redirect_url)
+
+
+@typed_endpoint
+def self_hosting_auth_json_endpoint(
+    request: HttpRequest,
+    user_profile: UserProfile,
+    *,
+    next_page: Optional[str] = None,
+) -> HttpResponse:
+    """
+    This endpoint is used by the desktop application. It makes an API request here,
+    expecting a JSON response with either the billing access link, or appropriate
+    error information.
+    """
+
+    redirect_url = self_hosting_auth_view_common(request, user_profile, next_page)
+
+    return json_success(request, data={"billing_access_url": redirect_url})

zproject/urls.py

@@ -92,7 +92,8 @@ from zerver.views.push_notifications import (
     add_apns_device_token,
     remove_android_reg_id,
     remove_apns_device_token,
-    self_hosting_auth_redirect,
+    self_hosting_auth_json_endpoint,
+    self_hosting_auth_redirect_endpoint,
     send_test_push_notification_api,
 )
 from zerver.views.reactions import add_reaction, remove_reaction
@@ -822,7 +823,15 @@ urls += [
 ]
 
 urls += [
-    path("self-hosted-billing/", self_hosting_auth_redirect, name="self_hosting_auth_redirect"),
+    path(
+        "self-hosted-billing/",
+        self_hosting_auth_redirect_endpoint,
+        name="self_hosting_auth_redirect_endpoint",
+    ),
+    rest_path(
+        "json/self-hosted-billing",
+        GET=self_hosting_auth_json_endpoint,
+    ),
 ]
 
 if not settings.CORPORATE_ENABLED:  # nocoverage