From c800951966221b9f8c370d80b7fb90661a075c1a Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Mon, 11 Dec 2023 14:30:07 +0100 Subject: [PATCH] remote_billing: Add some useful fields to Remote...User models. These are useful for auditing and follow what we have for UserProfile. And is_active will be used in the future when we add user deactivation. --- corporate/tests/test_remote_billing.py | 36 ++++++++-- corporate/views/remote_billing_page.py | 11 ++- ...erealmbillinguser_created_user_and_more.py | 72 +++++++++++++++++++ zilencer/models.py | 16 +++++ 4 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 zilencer/migrations/0050_preregistrationremoterealmbillinguser_created_user_and_more.py diff --git a/corporate/tests/test_remote_billing.py b/corporate/tests/test_remote_billing.py index 8d5a3dcb64..c4e10144ec 100644 --- a/corporate/tests/test_remote_billing.py +++ b/corporate/tests/test_remote_billing.py @@ -19,7 +19,13 @@ 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, RemoteRealmBillingUser, RemoteServerBillingUser +from zilencer.models import ( + PreregistrationRemoteRealmBillingUser, + PreregistrationRemoteServerBillingUser, + RemoteRealm, + RemoteRealmBillingUser, + RemoteServerBillingUser, +) if TYPE_CHECKING: from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse @@ -94,6 +100,10 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): self.assertEqual(remote_billing_user.user_uuid, user.uuid) self.assertEqual(remote_billing_user.email, user.delivery_email) + prereg_user = PreregistrationRemoteRealmBillingUser.objects.latest("id") + self.assertEqual(prereg_user.created_user, remote_billing_user) + self.assertEqual(remote_billing_user.date_joined, now) + # Now we should be redirected again to the /remote-billing-login/ endpoint # with a new signed_access_token. Now that the email has been confirmed, # and we have a RemoteRealmBillingUser entry, we'll be in the same position @@ -114,7 +124,8 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): if confirm_tos: params = {"tos_consent": "true"} - result = self.client_post(signed_auth_url, params, subdomain="selfhosting") + with time_machine.travel(now, tick=False): + 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 @@ -139,6 +150,8 @@ class RemoteBillingAuthenticationTest(BouncerTestCase): identity_dict, ) + self.assertEqual(remote_billing_user.last_login, now) + # It's up to the caller to verify further details, such as the exact redirect URL, # depending on the set up and intent of the test. return result @@ -543,6 +556,8 @@ class LegacyServerLoginTest(BouncerTestCase): identity_dict, ) + self.assertEqual(remote_billing_user.last_login, now) + return result def test_server_login_get(self) -> None: @@ -586,9 +601,11 @@ class LegacyServerLoginTest(BouncerTestCase): def test_server_login_success_with_no_plan(self) -> None: hamlet = self.example_user("hamlet") - result = self.execute_remote_billing_authentication_flow( - hamlet.delivery_email, hamlet.full_name, expect_tos=True, confirm_tos=True - ) + now = timezone_now() + with time_machine.travel(now, tick=False): + result = self.execute_remote_billing_authentication_flow( + hamlet.delivery_email, hamlet.full_name, expect_tos=True, confirm_tos=True + ) self.assertEqual(result.status_code, 302) self.assertEqual(result["Location"], f"/server/{self.uuid}/plans/") @@ -604,6 +621,15 @@ class LegacyServerLoginTest(BouncerTestCase): result = self.client_get(result["Location"], subdomain="selfhosting") self.assert_in_success_response([f"Upgrade {self.server.hostname}"], result) + # Verify the RemoteServerBillingUser and PreRegistrationRemoteServerBillingUser + # objects created in the process. + remote_billing_user = RemoteServerBillingUser.objects.latest("id") + self.assertEqual(remote_billing_user.email, hamlet.delivery_email) + + prereg_user = PreregistrationRemoteServerBillingUser.objects.latest("id") + self.assertEqual(prereg_user.created_user, remote_billing_user) + self.assertEqual(remote_billing_user.date_joined, now) + def test_server_login_success_consent_is_not_re_asked(self) -> None: hamlet = self.example_user("hamlet") result = self.execute_remote_billing_authentication_flow( diff --git a/corporate/views/remote_billing_page.py b/corporate/views/remote_billing_page.py index 8510e27c83..ef2be417a4 100644 --- a/corporate/views/remote_billing_page.py +++ b/corporate/views/remote_billing_page.py @@ -249,7 +249,8 @@ def remote_realm_billing_finalize_login( if full_name is not None: remote_user.full_name = full_name remote_user.tos_version = settings.TERMS_OF_SERVICE_VERSION - remote_user.save(update_fields=["full_name", "tos_version"]) + remote_user.last_login = timezone_now() + remote_user.save(update_fields=["full_name", "tos_version", "last_login"]) identity_dict["remote_billing_user_id"] = remote_user.id request.session["remote_billing_identities"] = {} @@ -370,6 +371,8 @@ def remote_realm_billing_from_login_confirmation_link( remote_realm=remote_realm, user_uuid=prereg_object.user_uuid, ) + prereg_object.created_user = remote_billing_user + prereg_object.save(update_fields=["created_user"]) identity_dict = RemoteBillingIdentityDict( user=RemoteBillingUserDict( @@ -608,6 +611,12 @@ def remote_billing_legacy_server_from_login_confirmation_link( email=prereg_object.email, remote_server=remote_server, ) + if created: + prereg_object.created_user = remote_billing_user + prereg_object.save(update_fields=["created_user"]) + + remote_billing_user.last_login = timezone_now() + remote_billing_user.save(update_fields=["last_login"]) # Refresh IdentityDict in the session. (Or create it # if the user came here e.g. in a different browser than they diff --git a/zilencer/migrations/0050_preregistrationremoterealmbillinguser_created_user_and_more.py b/zilencer/migrations/0050_preregistrationremoterealmbillinguser_created_user_and_more.py new file mode 100644 index 0000000000..43a4fa4b27 --- /dev/null +++ b/zilencer/migrations/0050_preregistrationremoterealmbillinguser_created_user_and_more.py @@ -0,0 +1,72 @@ +# Generated by Django 4.2.8 on 2023-12-11 12:55 + +import django.db.models.deletion +import django.utils.timezone +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zilencer", "0049_alter_remoterealmbillinguser_unique_together_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="preregistrationremoterealmbillinguser", + name="created_user", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="zilencer.remoterealmbillinguser", + ), + ), + migrations.AddField( + model_name="preregistrationremoterealmbillinguser", + name="date_joined", + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name="preregistrationremoteserverbillinguser", + name="created_user", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="zilencer.remoteserverbillinguser", + ), + ), + migrations.AddField( + model_name="preregistrationremoteserverbillinguser", + name="date_joined", + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name="remoterealmbillinguser", + name="date_joined", + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name="remoterealmbillinguser", + name="is_active", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="remoterealmbillinguser", + name="last_login", + field=models.DateTimeField(null=True), + ), + migrations.AddField( + model_name="remoteserverbillinguser", + name="date_joined", + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name="remoteserverbillinguser", + name="is_active", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="remoteserverbillinguser", + name="last_login", + field=models.DateTimeField(null=True), + ), + ] diff --git a/zilencer/models.py b/zilencer/models.py index 5dbdd28dc6..107fa3f716 100644 --- a/zilencer/models.py +++ b/zilencer/models.py @@ -161,6 +161,8 @@ class AbstractRemoteRealmBillingUser(models.Model): user_uuid = models.UUIDField() email = models.EmailField() + date_joined = models.DateTimeField(default=timezone_now) + class Meta: abstract = True @@ -168,6 +170,10 @@ class AbstractRemoteRealmBillingUser(models.Model): class RemoteRealmBillingUser(AbstractRemoteRealmBillingUser): full_name = models.TextField(default="") + last_login = models.DateTimeField(null=True) + + is_active = models.BooleanField(default=True) + TOS_VERSION_BEFORE_FIRST_LOGIN = UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN tos_version = models.TextField(default=TOS_VERSION_BEFORE_FIRST_LOGIN) @@ -188,12 +194,16 @@ class PreregistrationRemoteRealmBillingUser(AbstractRemoteRealmBillingUser): next_page = models.TextField(null=True) uri_scheme = models.TextField() + created_user = models.ForeignKey(RemoteRealmBillingUser, null=True, on_delete=models.SET_NULL) + class AbstractRemoteServerBillingUser(models.Model): remote_server = models.ForeignKey(RemoteZulipServer, on_delete=models.CASCADE) email = models.EmailField() + date_joined = models.DateTimeField(default=timezone_now) + class Meta: abstract = True @@ -201,6 +211,10 @@ class AbstractRemoteServerBillingUser(models.Model): class RemoteServerBillingUser(AbstractRemoteServerBillingUser): full_name = models.TextField(default="") + last_login = models.DateTimeField(null=True) + + is_active = models.BooleanField(default=True) + class Meta: unique_together = [ ("remote_server", "email"), @@ -214,6 +228,8 @@ class PreregistrationRemoteServerBillingUser(AbstractRemoteServerBillingUser): next_page = models.TextField(null=True) + created_user = models.ForeignKey(RemoteServerBillingUser, null=True, on_delete=models.SET_NULL) + class RemoteZulipServerAuditLog(AbstractRealmAuditLog): """Audit data associated with a remote Zulip server (not specific to a