From 250b52e3dcf63a764a51a3dd88306305656dd9e8 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 5 Dec 2023 01:31:50 +0100 Subject: [PATCH] remote_billing: Add a "confirm login" page in RemoteRealm auth flow. --- corporate/tests/test_remote_billing.py | 92 ++++++++++++++++-- corporate/views/remote_billing_page.py | 94 ++++++++++++++++++- ...m_billing_finalize_login_confirmation.html | 40 ++++++++ .../billing/remote_billing_confirmation.ts | 29 ++++++ web/styles/portico/billing.css | 4 + web/webpack.assets.json | 1 + .../migrations/0044_remoterealmbillinguser.py | 34 +++++++ zilencer/models.py | 12 +++ 8 files changed, 297 insertions(+), 9 deletions(-) create mode 100644 templates/corporate/remote_realm_billing_finalize_login_confirmation.html create mode 100644 web/src/billing/remote_billing_confirmation.ts create mode 100644 zilencer/migrations/0044_remoterealmbillinguser.py diff --git a/corporate/tests/test_remote_billing.py b/corporate/tests/test_remote_billing.py index 0cebedd8ce..8886c3057c 100644 --- a/corporate/tests/test_remote_billing.py +++ b/corporate/tests/test_remote_billing.py @@ -18,7 +18,7 @@ from zerver.lib.remote_server import send_realms_only_to_push_bouncer from zerver.lib.test_classes import BouncerTestCase from zerver.lib.timestamp import datetime_to_timestamp from zerver.models import UserProfile -from zilencer.models import RemoteRealm +from zilencer.models import RemoteRealm, RemoteRealmBillingUser if TYPE_CHECKING: from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse @@ -27,7 +27,11 @@ if TYPE_CHECKING: @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com") class RemoteBillingAuthenticationTest(BouncerTestCase): def execute_remote_billing_authentication_flow( - self, user: UserProfile, next_page: Optional[str] = None + self, + user: UserProfile, + next_page: Optional[str] = None, + expect_tos: bool = True, + confirm_tos: bool = True, ) -> "TestHttpResponse": now = timezone_now() @@ -42,10 +46,24 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): # We've received a redirect to an URL that will grant us an authenticated # session for remote billing. + signed_auth_url = result["Location"] with time_machine.travel(now, tick=False): - result = self.client_get(result["Location"], subdomain="selfhosting") - # When successful, we receive a final redirect. - self.assertEqual(result.status_code, 302) + result = self.client_get(signed_auth_url, subdomain="selfhosting") + # When successful, we see a confirmation page. + self.assertEqual(result.status_code, 200) + self.assert_in_success_response(["Log in to Zulip server billing"], result) + self.assert_in_success_response([user.realm.host], result) + + params = {} + if expect_tos: + self.assert_in_success_response(["I agree", "Terms of Service"], result) + if confirm_tos: + params = {"tos_consent": "true"} + + result = self.client_post(signed_auth_url, params, subdomain="selfhosting") + if result.status_code >= 400: + # Failures should be returned early so the caller can assert about them. + return result # Verify the authed data that should have been stored in the session. identity_dict = RemoteBillingIdentityDict( @@ -128,6 +146,62 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): self.assert_in_success_response(["Your remote user info:"], result) self.assert_in_success_response([desdemona.delivery_email], result) + @responses.activate + def test_remote_billing_authentication_flow_tos_consent_failure(self) -> None: + self.login("desdemona") + desdemona = self.example_user("desdemona") + + self.add_mock_response() + send_realms_only_to_push_bouncer() + + result = self.execute_remote_billing_authentication_flow( + desdemona, + expect_tos=True, + confirm_tos=False, + ) + + self.assert_json_error(result, "You must accept the Terms of Service to proceed.") + + @responses.activate + def test_remote_billing_authentication_flow_tos_consent_update(self) -> None: + self.login("desdemona") + desdemona = self.example_user("desdemona") + + self.add_mock_response() + send_realms_only_to_push_bouncer() + + with self.settings(TERMS_OF_SERVICE_VERSION="1.0"): + result = self.execute_remote_billing_authentication_flow( + desdemona, + expect_tos=True, + confirm_tos=True, + ) + + self.assertEqual(result.status_code, 302) + + remote_billing_user = RemoteRealmBillingUser.objects.last() + assert remote_billing_user is not None + self.assertEqual(remote_billing_user.user_uuid, desdemona.uuid) + self.assertEqual(remote_billing_user.tos_version, "1.0") + + # Now bump the ToS version. They need to agree again. + with self.settings(TERMS_OF_SERVICE_VERSION="2.0"): + result = self.execute_remote_billing_authentication_flow( + desdemona, + expect_tos=True, + confirm_tos=False, + ) + self.assert_json_error(result, "You must accept the Terms of Service to proceed.") + + result = self.execute_remote_billing_authentication_flow( + desdemona, + expect_tos=True, + confirm_tos=True, + ) + remote_billing_user.refresh_from_db() + self.assertEqual(remote_billing_user.user_uuid, desdemona.uuid) + self.assertEqual(remote_billing_user.tos_version, "2.0") + @responses.activate def test_remote_billing_authentication_flow_expired_session(self) -> None: now = timezone_now() @@ -174,7 +248,13 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): # flow via execute_remote_billing_authentication_flow with next_page="plans". # So let's test that and assert that we end up successfully re-authed on the /plans # page. - result = self.execute_remote_billing_authentication_flow(desdemona, next_page="plans") + result = self.execute_remote_billing_authentication_flow( + desdemona, + next_page="plans", + # ToS has already been confirmed earlier. + expect_tos=False, + confirm_tos=False, + ) self.assertEqual(result["Location"], f"/realm/{realm.uuid!s}/plans/") result = self.client_get(result["Location"], subdomain="selfhosting") self.assert_in_success_response(["Your remote user info:"], result) diff --git a/corporate/views/remote_billing_page.py b/corporate/views/remote_billing_page.py index 0625027e0a..c8e550a0f2 100644 --- a/corporate/views/remote_billing_page.py +++ b/corporate/views/remote_billing_page.py @@ -3,6 +3,7 @@ from typing import Any, Dict, Literal, Optional from django.conf import settings from django.core import signing +from django.core.exceptions import ObjectDoesNotExist from django.http import HttpRequest, HttpResponse, HttpResponseNotAllowed, HttpResponseRedirect from django.shortcuts import render from django.urls import reverse @@ -29,8 +30,13 @@ from zerver.lib.exceptions import JsonableError, MissingRemoteRealmError from zerver.lib.remote_server import RealmDataForAnalytics, UserDataForRemoteBilling from zerver.lib.response import json_success from zerver.lib.timestamp import datetime_to_timestamp -from zerver.lib.typed_endpoint import typed_endpoint -from zilencer.models import RemoteRealm, RemoteZulipServer, get_remote_server_by_uuid +from zerver.lib.typed_endpoint import PathOnly, typed_endpoint +from zilencer.models import ( + RemoteRealm, + RemoteRealmBillingUser, + RemoteZulipServer, + get_remote_server_by_uuid, +) billing_logger = logging.getLogger("corporate.stripe") @@ -83,10 +89,17 @@ def remote_realm_billing_entry( @self_hosting_management_endpoint +@typed_endpoint def remote_realm_billing_finalize_login( request: HttpRequest, - signed_billing_access_token: str, + *, + signed_billing_access_token: PathOnly[str], + tos_consent: Literal[None, "true"] = None, ) -> HttpResponse: + if request.method not in ["GET", "POST"]: + return HttpResponseNotAllowed(["GET", "POST"]) + tos_consent_given = tos_consent == "true" + # Sanity assert, because otherwise these make no sense. assert ( REMOTE_BILLING_SIGNED_ACCESS_TOKEN_VALIDITY_IN_SECONDS @@ -103,7 +116,82 @@ def remote_realm_billing_finalize_login( except signing.BadSignature: raise JsonableError(_("Invalid billing access token.")) + # Now we want to fetch (or create) the RemoteRealmBillingUser object implied + # by the IdentityDict. We'll use this: + # (1) If the user came here via just GET, we want to show them a confirmation + # page with the relevant info details before finalizing login. If they wish + # to proceed, they'll approve the form, causing a POST, bring us to case (2). + # (2) If the user came here via POST, we finalize login, using the info from the + # IdentityDict to update the RemoteRealmBillingUser object if needed. remote_realm_uuid = identity_dict["remote_realm_uuid"] + remote_server_uuid = identity_dict["remote_server_uuid"] + try: + remote_server = get_remote_server_by_uuid(remote_server_uuid) + remote_realm = RemoteRealm.objects.get(uuid=remote_realm_uuid, server=remote_server) + except ObjectDoesNotExist: + # These should definitely still exist, since the access token was signed + # pretty recently. (And we generally don't delete these at all.) + raise AssertionError + + user_dict = identity_dict["user"] + + user_email = user_dict["user_email"] + user_full_name = user_dict["user_full_name"] + user_uuid = user_dict["user_uuid"] + + assert ( + settings.TERMS_OF_SERVICE_VERSION is not None + ), "This is only run on the bouncer, which has ToS" + + try: + remote_user = RemoteRealmBillingUser.objects.get( + remote_realm=remote_realm, + user_uuid=user_uuid, + ) + tos_consent_needed = int(settings.TERMS_OF_SERVICE_VERSION.split(".")[0]) > int( + remote_user.tos_version.split(".")[0] + ) + except RemoteRealmBillingUser.DoesNotExist: + # This is the first time this user is logging in, so ToS consent needed. + tos_consent_needed = True + + if request.method == "GET": + context = { + "remote_server_uuid": remote_server_uuid, + "remote_realm_uuid": remote_realm_uuid, + "remote_realm_host": remote_realm.host, + "user_email": user_email, + "user_full_name": user_full_name, + "tos_consent_needed": tos_consent_needed, + "action_url": reverse( + remote_realm_billing_finalize_login, args=(signed_billing_access_token,) + ), + } + return render( + request, + "corporate/remote_realm_billing_finalize_login_confirmation.html", + context=context, + ) + + assert request.method == "POST" + + if tos_consent_needed and not tos_consent_given: + # This shouldn't be possible without tampering with the form, so we + # don't need a pretty error. + raise JsonableError(_("You must accept the Terms of Service to proceed.")) + + remote_user, created = RemoteRealmBillingUser.objects.get_or_create( + defaults={"full_name": user_full_name, "email": user_email}, + remote_realm=remote_realm, + user_uuid=user_uuid, + ) + + # The current approach is to just update the email and full_name + # based on the info provided by the remote server during auth. + remote_user.email = user_email + remote_user.full_name = user_full_name + remote_user.tos_version = settings.TERMS_OF_SERVICE_VERSION + remote_user.save(update_fields=["email", "full_name", "tos_version"]) request.session["remote_billing_identities"] = {} request.session["remote_billing_identities"][ diff --git a/templates/corporate/remote_realm_billing_finalize_login_confirmation.html b/templates/corporate/remote_realm_billing_finalize_login_confirmation.html new file mode 100644 index 0000000000..61c24cbf40 --- /dev/null +++ b/templates/corporate/remote_realm_billing_finalize_login_confirmation.html @@ -0,0 +1,40 @@ +{% extends "zerver/portico.html" %} +{% set entrypoint = "upgrade" %} + +{% block title %} +{{ _("Billing login confirmation") }} | Zulip +{% endblock %} + +{% block portico_content %} +
+
+
+

Log in to Zulip server billing for {{ remote_realm_host }}

+
+
+

Click Continue to log in to Zulip server + billing with the account below.

+ Full name: {{ user_full_name }}
+ Email: {{ user_email }}
+
+ {{ csrf_input }} + {% if tos_consent_needed %} +
+ +
+ {% endif %} +
+ +
+
+
+
+
+{% endblock %} diff --git a/web/src/billing/remote_billing_confirmation.ts b/web/src/billing/remote_billing_confirmation.ts new file mode 100644 index 0000000000..3031038b31 --- /dev/null +++ b/web/src/billing/remote_billing_confirmation.ts @@ -0,0 +1,29 @@ +import $ from "jquery"; + +export function initialize(): void { + $("#remote-realm-confirm-login-form").find(".loader").hide(); + + $("#remote-realm-confirm-login-form").validate({ + errorClass: "text-error", + wrapper: "div", + submitHandler(form) { + $("#remote-realm-confirm-login-form").find(".loader").show(); + $("#remote-realm-confirm-login-button .remote-realm-confirm-login-button-text").hide(); + + form.submit(); + }, + invalidHandler() { + $("#remote-realm-confirm-login-form .alert.alert-error").remove(); + }, + showErrors(error_map) { + if (error_map.id_terms) { + $("#remote-realm-confirm-login-form .alert.alert-error").remove(); + } + this.defaultShowErrors!(); + }, + }); +} + +$(() => { + initialize(); +}); diff --git a/web/styles/portico/billing.css b/web/styles/portico/billing.css index 411a876fd5..295797c280 100644 --- a/web/styles/portico/billing.css +++ b/web/styles/portico/billing.css @@ -674,3 +674,7 @@ input[name="licenses"] { #upgrade-page-details #due-today-for-future-update-wrapper { display: none; } + +#remote-realm-confirm-login-form .text-error { + margin-bottom: 15px; +} diff --git a/web/webpack.assets.json b/web/webpack.assets.json index cafc6298a1..688f632949 100644 --- a/web/webpack.assets.json +++ b/web/webpack.assets.json @@ -28,6 +28,7 @@ "./src/billing/upgrade", "jquery-validation", "./src/billing/legacy_server_login", + "./src/billing/remote_billing_confirmation", "./styles/portico/billing.css" ], "billing-event-status": [ diff --git a/zilencer/migrations/0044_remoterealmbillinguser.py b/zilencer/migrations/0044_remoterealmbillinguser.py new file mode 100644 index 0000000000..d2f30e4722 --- /dev/null +++ b/zilencer/migrations/0044_remoterealmbillinguser.py @@ -0,0 +1,34 @@ +# Generated by Django 4.2.7 on 2023-12-05 01:15 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zilencer", "0043_remotepushdevicetoken_remote_realm"), + ] + + operations = [ + migrations.CreateModel( + name="RemoteRealmBillingUser", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("user_uuid", models.UUIDField()), + ("full_name", models.TextField(default="")), + ("email", models.EmailField(max_length=254)), + ("tos_version", models.TextField(default="-1")), + ( + "remote_realm", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="zilencer.remoterealm" + ), + ), + ], + ), + ] diff --git a/zilencer/models.py b/zilencer/models.py index 8d299980a8..63de4ad625 100644 --- a/zilencer/models.py +++ b/zilencer/models.py @@ -150,6 +150,18 @@ class RemoteRealm(models.Model): return f"{self.host} {str(self.uuid)[0:12]}" +class RemoteRealmBillingUser(models.Model): + remote_realm = models.ForeignKey(RemoteRealm, on_delete=models.CASCADE) + + # The .uuid of the UserProfile on the remote server + user_uuid = models.UUIDField() + full_name = models.TextField(default="") + email = models.EmailField() + + TOS_VERSION_BEFORE_FIRST_LOGIN = UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN + tos_version = models.TextField(default=TOS_VERSION_BEFORE_FIRST_LOGIN) + + class RemoteZulipServerAuditLog(AbstractRealmAuditLog): """Audit data associated with a remote Zulip server (not specific to a realm). Used primarily for tracking registration and billing