From dbe35767060bbf5e3fa78fd67e72b15780e7bb2e Mon Sep 17 00:00:00 2001
From: Rishi Gupta <rishig@zulipchat.com>
Date: Fri, 1 Dec 2017 13:37:33 -0800
Subject: [PATCH] registration: Enforce realm is None only if realm_creation.

Commit d4ee3023 and its parent have the history behind this code.

Since d4ee3023^, all new PreregistrationUser objects, except those for
realm creation, have a non-None `realm`.  Since d4ee3023, any legacy
PreregistrationUsers, with a `realm` of None despite not being for
realm creation, are treated as expired.  Now, we ignore them
completely, and remove any that exist from the database.

The user-visible effect is to change the error message for
registration (or invitation) links created before d4ee3023^ to be
"link does not exist", rather than "link expired".

This change will at most affect users upgrading straight from 1.7 or
earlier to 1.8 (rather than from 1.7.1), but I think that's not much
of a concern (such installations are probably long-running
installations, without many live registration or invitation links).

[greg: tweaked commit message]
---
 .../0126_prereg_remove_users_without_realm.py | 23 +++++++++++++++++++
 zerver/tests/test_signup.py                   | 20 ----------------
 zerver/views/registration.py                  |  4 +---
 3 files changed, 24 insertions(+), 23 deletions(-)
 create mode 100644 zerver/migrations/0126_prereg_remove_users_without_realm.py

diff --git a/zerver/migrations/0126_prereg_remove_users_without_realm.py b/zerver/migrations/0126_prereg_remove_users_without_realm.py
new file mode 100644
index 0000000000..c1eba8640e
--- /dev/null
+++ b/zerver/migrations/0126_prereg_remove_users_without_realm.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.6 on 2017-11-30 04:58
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+def remove_prereg_users_without_realm(
+        apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
+    prereg_model = apps.get_model("zerver", "PreregistrationUser")
+    prereg_model.objects.filter(realm=None, realm_creation=False).delete()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0125_realm_max_invites'),
+    ]
+
+    operations = [
+        migrations.RunPython(remove_prereg_users_without_realm,
+                             reverse_code=migrations.RunPython.noop),
+    ]
diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py
index 1f6183c363..7aa1b8a84e 100644
--- a/zerver/tests/test_signup.py
+++ b/zerver/tests/test_signup.py
@@ -1797,26 +1797,6 @@ class UserSignUpTest(ZulipTestCase):
              'from_confirmation': '1'},  subdomain="zephyr")
         self.assert_in_success_response(["We couldn't find your confirmation link"], result)
 
-    def test_failed_signup_due_to_empty_realm_in_prereg_user(self) -> None:
-        """
-        Largely to test a transitional state, where we started requiring the
-        realm in PreregistrationUser (if realm_creation is False), and wanted
-        to make sure we had properly disabled any existing confirmation links that
-        didn't have the realm set.
-        """
-        email = "newuser@zulip.com"
-        password = "password"
-        self.client_post('/accounts/home/', {'email': email})
-        PreregistrationUser.objects.update(realm=None)
-        result = self.client_post(
-            '/accounts/register/',
-            {'password': password,
-             'key': find_key_by_email(email),
-             'terms': True,
-             'full_name': "New User",
-             'from_confirmation': '1'})
-        self.assert_in_success_response(["The confirmation link has expired or been deactivated."], result)
-
     def test_failed_signup_due_to_restricted_domain(self) -> None:
         realm = get_realm('zulip')
         realm.invite_required = False
diff --git a/zerver/views/registration.py b/zerver/views/registration.py
index e70ed62bac..a657177b0b 100644
--- a/zerver/views/registration.py
+++ b/zerver/views/registration.py
@@ -83,9 +83,7 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
         realm = None
     else:
         realm = get_realm(get_subdomain(request))
-        if prereg_user.realm is None:
-            return render(request, 'confirmation/link_expired.html')
-        if prereg_user.realm != realm:
+        if realm is None or realm != prereg_user.realm:
             return render(request, 'confirmation/link_does_not_exist.html')
 
     if realm and not email_allowed_for_realm(email, realm):