From fc247cba3f63f70e776c55932cb20b303238be6c Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sat, 6 Jan 2024 20:31:41 +0100 Subject: [PATCH] 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 | 52 +++++++++++++++++ web/src/desktop_integration.js | 31 +++++++++++ web/src/ui_init.js | 2 + web/templates/gear_menu_popover.hbs | 2 +- zerver/views/push_notifications.py | 77 +++++++++++++++++++------- zproject/urls.py | 13 ++++- 6 files changed, 154 insertions(+), 23 deletions(-) diff --git a/corporate/tests/test_remote_billing.py b/corporate/tests/test_remote_billing.py index e7548e43e7..4a8d136be1 100644 --- a/corporate/tests/test_remote_billing.py +++ b/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 diff --git a/web/src/desktop_integration.js b/web/src/desktop_integration.js index 047cd005d0..3cf194b651 100644 --- a/web/src/desktop_integration.js +++ b/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"}), + }); + } + }, + }); + }); +} diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 4a58a6a693..fdf580366d 100644 --- a/web/src/ui_init.js +++ b/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"); } diff --git a/web/templates/gear_menu_popover.hbs b/web/templates/gear_menu_popover.hbs index 39f20bb1a8..be4d909d7c 100644 --- a/web/templates/gear_menu_popover.hbs +++ b/web/templates/gear_menu_popover.hbs @@ -135,7 +135,7 @@ {{/if}} {{#if show_remote_billing }}
  • - + {{t 'Plan management' }}
    • diff --git a/zerver/views/push_notifications.py b/zerver/views/push_notifications.py index 7e7e799dc1..f18a3ae264 100644 --- a/zerver/views/push_notifications.py +++ b/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 -@typed_endpoint -def self_hosting_auth_redirect( - request: HttpRequest, - *, - next_page: Optional[str] = None, -) -> HttpResponse: - if not uses_notification_bouncer(): # nocoverage - return render(request, "404.html", status=404) +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")) - user = request.user - assert user.is_authenticated - assert isinstance(user, UserProfile) - if not user.has_billing_access: # nocoverage + if not user_profile.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, - email=user.delivery_email, - full_name=user.full_name, + uuid=user_profile.uuid, + email=user_profile.delivery_email, + 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}) diff --git a/zproject/urls.py b/zproject/urls.py index 8384480e39..dd23ed8e00 100644 --- a/zproject/urls.py +++ b/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