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.
This commit is contained in:
Mateusz Mandera 2024-01-06 20:31:41 +01:00 committed by Tim Abbott
parent aea290a278
commit ba50faced4
6 changed files with 154 additions and 23 deletions

View File

@ -23,6 +23,7 @@ from corporate.models import (
get_customer_by_remote_server, get_customer_by_remote_server,
) )
from corporate.views.remote_billing_page import generate_confirmation_link_for_server_deactivation from corporate.views.remote_billing_page import generate_confirmation_link_for_server_deactivation
from zerver.lib.exceptions import RemoteRealmServerMismatchError
from zerver.lib.remote_server import send_server_data_to_push_bouncer from zerver.lib.remote_server import send_server_data_to_push_bouncer
from zerver.lib.test_classes import BouncerTestCase from zerver.lib.test_classes import BouncerTestCase
from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.timestamp import datetime_to_timestamp
@ -169,6 +170,57 @@ class RemoteRealmBillingTestCase(BouncerTestCase):
return result 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") @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com")
class RemoteBillingAuthenticationTest(RemoteRealmBillingTestCase): class RemoteBillingAuthenticationTest(RemoteRealmBillingTestCase):
@responses.activate @responses.activate

View File

@ -2,6 +2,8 @@ import $ from "jquery";
import * as browser_history from "./browser_history"; import * as browser_history from "./browser_history";
import * as channel from "./channel"; 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 message_store from "./message_store";
import * as narrow from "./narrow"; import * as narrow from "./narrow";
import * as stream_data from "./stream_data"; 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"}),
});
}
},
});
});
}

View File

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

View File

@ -135,7 +135,7 @@
{{/if}} {{/if}}
{{#if show_remote_billing }} {{#if show_remote_billing }}
<li class="link-item navbar-dropdown-menu-inner-list-item"> <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' }} <i class="navbar-dropdown-icon zulip-icon zulip-icon-rocket" aria-hidden="true"></i> {{t 'Plan management' }}
</a> </a>
</li> </li>

View File

@ -12,6 +12,7 @@ from zerver.lib.exceptions import (
MissingRemoteRealmError, MissingRemoteRealmError,
OrganizationOwnerRequiredError, OrganizationOwnerRequiredError,
RemoteRealmServerMismatchError, RemoteRealmServerMismatchError,
ResourceNotFoundError,
) )
from zerver.lib.push_notifications import ( from zerver.lib.push_notifications import (
InvalidPushDeviceTokenError, InvalidPushDeviceTokenError,
@ -118,32 +119,25 @@ def send_test_push_notification_api(
return json_success(request) 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, # 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 # 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. # error may be sufficient.
raise OrganizationOwnerRequiredError 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( 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 = { post_data = {
@ -164,12 +158,55 @@ def self_hosting_auth_redirect(
# Upload realm info and re-try. It should work now. # Upload realm info and re-try. It should work now.
send_server_data_to_push_bouncer(consider_usage_statistics=False) send_server_data_to_push_bouncer(consider_usage_statistics=False)
result = send_to_push_bouncer("POST", "server/billing", post_data) 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 if result["result"] != "success": # nocoverage
raise JsonableError(_("Error returned by the bouncer: {result}").format(result=result)) raise JsonableError(_("Error returned by the bouncer: {result}").format(result=result))
redirect_url = result["billing_access_url"] redirect_url = result["billing_access_url"]
assert isinstance(redirect_url, str) 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) 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})

View File

@ -92,7 +92,8 @@ from zerver.views.push_notifications import (
add_apns_device_token, add_apns_device_token,
remove_android_reg_id, remove_android_reg_id,
remove_apns_device_token, remove_apns_device_token,
self_hosting_auth_redirect, self_hosting_auth_json_endpoint,
self_hosting_auth_redirect_endpoint,
send_test_push_notification_api, send_test_push_notification_api,
) )
from zerver.views.reactions import add_reaction, remove_reaction from zerver.views.reactions import add_reaction, remove_reaction
@ -822,7 +823,15 @@ urls += [
] ]
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 if not settings.CORPORATE_ENABLED: # nocoverage